fix(policy): resolve query-only condition keys from the query alone

[harshavardhana]

Summary

A condition key documented as coming from a query parameter only could be
satisfied by a request header instead. Those keys now resolve from the exact
name or not at all.

What the fallback is for, and where it goes wrong

getValuesByKey looks a key up by Name() — the lowercase form left after the
service prefix is stripped — and falls back to the canonical header spelling:

name := key.Name()                              // "s3:prefix" -> "prefix"
if values, found := m[name]; found { return values }
return m[http.CanonicalHeaderKey(name)]         // "Prefix"

That fallback is load-bearing for header-sourced keys. s3:x-amz-acl
becomes x-amz-acl, while a server records the header it came from
canonically as X-Amz-Acl, so without the fallback those keys never resolve.

Applied to a query-only key it is a bypass. Five keys document their value as
coming from a query parameter only:

Key	Name()	fallback reached
s3:prefix	prefix	Prefix
s3:delimiter	delimiter	Delimiter
s3:max-keys	max-keys	Max-Keys
memory:prefix	prefix	Prefix
memory:max-keys	max-keys	Max-Keys

A server merges request headers into the condition values, so a caller can
supply Prefix: allowed/ as a header with no prefix query parameter at
all. The condition is then evaluated against allowed/ while the listing runs
with no prefix — the condition checking one value and the request being served
with another. Measured against a real ListObjectsV2 request and a policy
whose s3:ListBucket grant is bounded by StringLike s3:prefix:

                                 | handler lists     | ListBucket allowed
query prefix inside the grant    | "allowed/"        | true    (correct)
query prefix outside the grant   | "other/"          | false   (correct)
HEADER prefix, no query prefix   | ""  (whole bucket)| true  <- before
HEADER prefix, no query prefix   | ""  (whole bucket)| false <- after

A present-but-empty ?prefix= was already safe: the exact name is then found
and the fallback never runs. Only an entirely absent query parameter reached it.

Changes

• KeyName.IsQueryOnly() plus the queryOnlyKeys set naming the five keys.
• getValuesByKey skips the canonical-header fallback for a query-only key.
  Header-sourced keys are untouched.

Testing

go build ./... and go test ./... pass.

• TestQueryOnlyKeyIgnoresAHeaderValue covers all five keys against a header
  value, each query-only key against its exact query name, and two
  header-sourced keys in both spellings, so the fallback is pinned as still
  working where it is needed.
• TestQueryOnlyKeysAreTheDocumentedSet asserts the set both ways, so a key
  added to one and not the other fails.

Verified end to end against an unmodified AIStor build with a local replace:
the header-only row flips to denied and the query rows are unchanged.

Scope

This changes condition evaluation for every policy using these keys, not one
feature, which is why it is separate from #266 — it is reviewable and
backportable on its own. Say the word and I will fold it into #266 instead.

Summary by CodeRabbit

• Bug Fixes
  • Query-only condition values are now resolved exclusively from their exact query parameter names.
  • Prevented query-only conditions from incorrectly matching equivalent HTTP header names.
  • Preserved canonical header matching for conditions that support header-based values.
  • Added coverage to verify documented query-only keys and their lookup behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 34193b42-23f2-4444-bf75-70ef233ea0ca

📥 Commits

Reviewing files that changed from the base of the PR and between b5f0782 and e51a0e6.

📒 Files selected for processing (3)

• policy/condition/keyname.go
• policy/condition/value.go
• policy/condition/value_test.go

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

---

📝 Walkthrough

Walkthrough

The change marks five condition keys as query-only. Value lookup now skips canonical-header fallback for these keys. Tests verify query-name lookup, header-key behavior, and the documented key set.

Changes

Query-only condition handling

Layer / File(s)	Summary
Query-only key classification policy/condition/keyname.go	Adds the query-only key set and the KeyName.IsQueryOnly() method.
Lookup enforcement and tests policy/condition/value.go, policy/condition/value_test.go	Stops canonical-header fallback for query-only keys. Tests verify exact query names, header-sourced keys, and set membership.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e51a0

The query-only lookup behavior is covered by the described implementation and tests, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: resolving query-only policy condition keys from query parameters instead of headers.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

---

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 query trail
Five keys stay on the proper rail
Headers no longer change their way
Exact names guide the lookup day
Tests watch each path and set
Clean conditions, no regret}

_{Comment @coderabbitai help to get the list of available commands.}

[harshavardhana]

Closing: not worth the blast radius.

The behaviour is real -- a Prefix: header satisfies a condition on
s3:prefix when the query parameter is absent -- but what it exposes is key
names, not object contents, and that exposure is already documented and
accepted in AIStor's agent design: the two resource-bound Deny statements
are what confine an agent on the object path, never the s3:prefix
condition.

Narrowing getValuesByKey changes condition evaluation for every policy in
the product that uses these five keys, which is a lot of surface to move for a
leak that is accepted by design where it matters.