[taran-p]

MatchSimple let any ? match nothing once the name ran out, so a
principal such as "?alice" matched the anonymous account and an Allow
statement with it granted public access. Only a ? followed by nothing
but ? or * is optional now; any other ? needs exactly one character.

Summary by CodeRabbit

• Bug Fixes
  • Improved wildcard matching in access policies. Anonymous GetObject requests are correctly denied when principal patterns include question marks, and principal matching now handles empty values and optional wildcard characters more accurately.
  • Corrected matching behavior for patterns that combine ? and *, including cases where the input is empty or its length does not meet the pattern’s requirements.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The wildcard matcher now treats question marks as optional only in a trailing run of ? and *; question marks before literals remain required. Tests cover empty and nonempty matches, principal matching, and anonymous GetObject policy decisions.

Changes: Wildcard matching

Layer / File(s)	Summary
Trailing wildcard matching wildcard/match.go, wildcard/match_equivalence_test.go, wildcard/match_test.go	The matcher limits optional question marks to a trailing run of ? and *. Tests cover empty inputs, exact-length matches, and patterns with literals.
Principal and bucket-policy tests policy/principal_test.go, policy/bucket-policy_test.go	Tests cover wildcard matches against empty and nonempty principals. Bucket-policy tests check that anonymous GetObject requests are denied for three principal patterns.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: klauspost, 0xmalvee

Merge Risk: Minimal · up to 62bf6

The change prevents literal-containing principal patterns from matching anonymous requests while preserving documented trailing-wildcard behavior. No actionable merge-blocking risk was identified; normal test checks should pass before merging.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: MatchSimple now treats a ? as optional only when it is a trailing run of ? or *. The raw summary confirms this change in wildcard/match.go, and the PR objectives…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: MatchSimple now treats a ? as optional only when it is a trailing run of ? or *. The raw summary confirms this change in wildcard/match.go, and the PR objectives describe the same fix for ?alice matching the anonymous account. The title is specific, short, and matches the changeset.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 5 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the patterns at the gate,
A question mark now knows its place.
The empty string slips through a star,
While literals stay just where they are.
The burrow’s tests all pass with grace.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 28c8af18-2da6-40b7-b6c9-2e441dd6aa7c

Commits

Reviewing files that changed from the base of the PR and between f76516c and 62bf6a0.

Files selected for processing: 5

• policy/bucket-policy_test.go
• policy/principal_test.go
• wildcard/match.go
• wildcard/match_equivalence_test.go
• wildcard/match_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.