Repository navigation
feat(s3): add the /s3/bucket/policy command - #80
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| ETag string `cborgen:"etag" dagjsongen:"etag"` | ||
| // Policy is the stored policy document in its canonical JSON form. It is | ||
| // set only for a GetBucketPolicy. | ||
| Policy []byte `cborgen:"policy" dagjsongen:"policy"` |
There was a problem hiding this comment.
You could move Hilt's Policy here and return it instead of []byte. You'd have to define cborgen: tags on the type, and generate the CBOR marshal/unmarshal code but you'd then avoid the implicit encoding here (documented only in the comment) and it would be a smaller payload to transfer from Hilt to Ingot (since you'd be transferring CBOR not JSON).
If you ever actually needed to parse the policy you'd avoid having to separately do that also, since it would be decoded as part of the request (I realise you don't need to do that right now in Ingot).
Note that if you do do this you would have to marshal it to JSON in Ingot to return it to the client.
| Policy []byte `cborgen:"policy" dagjsongen:"policy"` | |
| Policy *Policy `cborgen:"policy,omitempty" dagjsongen:"policy,omitempty"` |
There was a problem hiding this comment.
According to Claude doing that would involve wiriting a custom CBOR codec in order to handle Principal value that can be * or []string.
What's your take on that? Is it something worth doing?
There was a problem hiding this comment.
Done. PolicyOK.Policy is now a *BucketPolicy, with Statement and Principal moved here from Hilt and generated CBOR/DAG-JSON codecs (12ca555). The type is named BucketPolicy because bucket.Policy is already the command binding.
- Hilt aliases
Statement,PrincipalandEffect, and declaresPolicyas a defined type overBucketPolicy. That keeps Hilt's strictUnmarshalJSONthere, since its checks use Hilt's action vocabulary (fil-forge/hilt@ee26a9b on #96). - Ingot marshals the policy to JSON for the client. The bytes are the same as the canonical document Hilt stores (fil-forge/ingot@4b592cc on #194). The local policies e2e suite passes, 103/103.
PolicyArguments.Bodyis still the raw document, because Hilt validates what the client sent and returnsInvalidBucketPolicyfor it.
There was a problem hiding this comment.
Hilt now uses the libforge types directly, without aliases. The strict parsing and the structural checks moved here too: BucketPolicy.UnmarshalJSON, WellFormed, and the policy action allowlist (PolicyAction, PolicyActions, PolicyWildcard). So json.Unmarshal into a BucketPolicy runs the same checks in Hilt, Ingot or anywhere else (64602c9). Hilt keeps the canonical ordering, the ETag, Validate against the bucket and the tenant's principals, and evaluation (fil-forge/hilt@24fef2d). Ingot takes the error name from libforge (fil-forge/ingot@1b1c54d). The policies e2e suite still passes, 103/103.
There was a problem hiding this comment.
Following your note on StatementModel: Statement is one generated type again (fe92838). Actions and Resources are StringSet (map[string]commands.Unit) and Principal is type Principal StringSet, the wildcard being its single member "*". Both generators take the named map types inline, so the CBOR and DAG-JSON codecs are generated and the bytes are unchanged (checked against 66e4414; Hilt's pinned ETags pass untouched). The hand-written part is only the AWS JSON form: StringSet writes a sorted list and reads a string or a list, Principal writes "*" or {"AWS": [user ARNs]}. StatementModel and its codec are gone. Hilt ba64314 reads members and clones maps where it mutates; ingot 423278a re-pins.
| type PolicyOK struct { | ||
| // ETag is the tag of the bucket's policy after the operation. It is empty | ||
| // after a DeleteBucketPolicy. | ||
| ETag string `cborgen:"etag" dagjsongen:"etag"` |
There was a problem hiding this comment.
Do we need the etag? Isn't it just the hash of the canonical encoding of the policy? i.e. hash(policy.marshalDagJson()) - Ingot should be able to generate this.
There was a problem hiding this comment.
I don't mind generating in Ingot, but we do however save the etag in hilt.
There was a problem hiding this comment.
Given that we're using versitygw handlers now generating the Etag would require us to overwrite the handlers, so I passing the Etag from Hilt over to Ingot is the way to go in this case.
There was a problem hiding this comment.
Closing the loop here: the ETag is now bucket.ETag(doc) in this package (c06edf8), the CID of the normalized document's DAG-JSON, and PolicyOK no longer carries one. Hilt stores and checks it, Ingot computes it from the document it serves (ingot aec3add). The earlier objection about versitygw's handlers did not hold; Ingot already wrapped them.
Ingot forwards S3 GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy requests as one command; the request's method selects the operation. A service key holding s3:GetBucketPolicy, s3:PutBucketPolicy or s3:DeleteBucketPolicy reaches it, a principal-bound key is refused. A PUT's body must hash to the request's signed payload hash, so nothing on the path can replace the document. If-Match and If-None-Match: * are optional signed headers; without one the write is unconditional, as on AWS. The policy service's rejections (PolicyNotFound, InvalidPrecondition, PreconditionFailed, ConcurrentChange) become receipt failures for Ingot to render. Temporary pin to libforge branch srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80), re-pin before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PolicyOK.Policy is now a *BucketPolicy encoded in CBOR with the rest of
the result, instead of the policy's canonical JSON bytes. BucketPolicy,
Statement and Principal move here from Hilt. Principal keeps its Go
shape {All, IDs} on the wire, so the generated codecs cover it; its JSON
methods give the AWS form ("*" or {"AWS": [user ARNs]}), which Ingot
writes back to the S3 client. PolicyArguments.Body stays the raw
document: Hilt validates what the client sent.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BucketPolicy.UnmarshalJSON is now the strict parser Hilt had: exact property names, no unknown fields, a null Sid refused, and WellFormed's structural checks. The policy action allowlist (PolicyAction, PolicyActions, PolicyWildcard), the bucket and object ARN helpers, MaxSetSize and ErrInvalidPolicy (InvalidBucketPolicy) move here with it, so Hilt and Ingot use these types directly. Ordering statements into canonical form stays in Hilt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| const MaxSetSize = 4096 | ||
|
|
||
| // policyActions is the allowlist of permissions a bucket policy statement may | ||
| // carry, sorted. The rest of the S3 permission vocabulary stays out on |
There was a problem hiding this comment.
I would sort this in code. This comment is easy to miss.
There was a problem hiding this comment.
Done in 66e4414: the allowlist goes through sorted() at init, and TestPolicyActions already checks PolicyActions() is increasing, so an out-of-order entry fails the build.
| var ErrInvalidPolicy = errors.New(InvalidPolicyErrorName, "invalid bucket policy") | ||
|
|
||
| // UserARN returns the ARN a document names principal id by. | ||
| func UserARN(id string) string { return UserARNPrefix + id } |
There was a problem hiding this comment.
Is there any escaping we need to do here?
There was a problem hiding this comment.
No escaping needed. The id is placed after the fixed prefix verbatim and recovered by an exact prefix cut, so nothing parses past the prefix; the JSON escaping is encoding/json's on both sides. Hilt's ValidID also refuses / ; , ? %, * and dot segments when a principal is created. 66e4414 adds a round-trip test with an id holding /, : and a quote.
| // ParseS3ARN splits arn:aws:s3:::<bucket> or arn:aws:s3:::<bucket>/* into the | ||
| // bucket and whether it names the bucket's objects. Any other shape, a key | ||
| // prefix or a wildcard in the bucket name included, is not ok. | ||
| func ParseS3ARN(arn string) (bucket string, objects bool, ok bool) { |
There was a problem hiding this comment.
I would return an error not a bool so that you can give more context why the arn was invalid.
There was a problem hiding this comment.
Done in 66e4414: ParseS3ARN returns an error naming the reason (not an S3 ARN, empty bucket name, or a bucket name holding /, * or ?), and WellFormed wraps it with the statement index and ErrInvalidPolicy.
| // ParseS3ARN splits arn:aws:s3:::<bucket> or arn:aws:s3:::<bucket>/* into the | ||
| // bucket and whether it names the bucket's objects. Any other shape, a key | ||
| // prefix or a wildcard in the bucket name included, is not ok. | ||
| func ParseS3ARN(arn string) (bucket string, objects bool, ok bool) { |
There was a problem hiding this comment.
Added ParseUserARN in 66e4414, the counterpart of ParseS3ARN and of UserARN; Principal's JSON decoder uses it. It accepts any non-empty id after the prefix, since which ids exist is Hilt's business (ValidID). Hilt 0d878dbd and ingot 17c5c28 re-pin.
The IPLD form of a Statement (CBOR and DAG-JSON) is StatementModel: the principal, action and resource lists as maps from member to commands.Unit, the wildcard principal the single member "*". Statement's hand-written codec converts to and from the generated model, so decoding sorts and dedupes each list. Principal no longer has a codec of its own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ParseS3ARN says why an ARN is refused instead of returning false, and ParseUserARN is its counterpart for principals, used by Principal's JSON decoder. The policy action allowlist is sorted at init rather than by hand. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Statement is one generated type again: Actions and Resources are StringSet
(map[string]commands.Unit) and Principal is a StringSet whose single member
"*" names every principal. The IPLD codecs are generated; the hand-written
part is the AWS JSON form, where a set is a sorted list and a principal is
"*" or {"AWS": [user ARNs]}. StatementModel and its codec are gone.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Normalize orders a policy's statements by their CID and drops duplicates; ETag is the CID of the normalized document's DAG-JSON. Hilt stores and checks the tag, Ingot answers clients with it, so the receipt of /s3/bucket/policy no longer carries one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| } | ||
| keyedStatements := make([]keyed, 0, len(d.Statements)) | ||
| for _, st := range d.Statements { | ||
| keyedStatements = append(keyedStatements, keyed{key: StatementCID(st).String(), st: st}) |
There was a problem hiding this comment.
Might be slightly simpler to use a map[cid.Cid]Statement to dedupe, and then sort the keys and create the []Statement from them.
There was a problem hiding this comment.
Minor: The version and IPLD codec in the CID are always the same and redundant. You're really sorting by the SHA2-256 digest of the statement (by spec - unfortunately there's no structural ordering here) so you could probably do a little less work by having a func StatementDigest(st Statement) multihash.Multihash instead. Note you can sort by bytes (which is what a multihash is - []byte), you don't have to encode to string.
|
|
||
| // BucketPolicy is a bucket policy document. Its JSON encoding is the AWS | ||
| // bucket policy document. | ||
| type BucketPolicy struct { |
There was a problem hiding this comment.
The package name bucket stutters as bucket.BucketPolicy which is a general Go style guideline. I'd rename to Policy.
There was a problem hiding this comment.
Gotcha! I believe Claude chose this name in order to avoid the clash with some internal Hilt types which is now not the case given that Hilt uses types defined in libforge. Will refactor this.
There was a problem hiding this comment.
Renamed to PolicyDocument in 491d329 (AWS's own term for it). Policy itself stays the command binding: every binding in commands/ is named after the last segment of its path (Create, Info, Authorize, …), and Hilt and Ingot reach it as bucket.Policy.Command / .Handler, so the type took the longer name rather than the binding leaving the pattern.
| // principal, action and resource sets need nothing, being sets. Two policies | ||
| // that differ only in statement order or in duplicate statements normalize | ||
| // to the same value. | ||
| func Normalize(d BucketPolicy) BucketPolicy { |
There was a problem hiding this comment.
I'd model []Statement as type StatementSet []Statement and it would have Marshal* and Unmarshal* methods that sort the list appropriately.
You can add or remove from the list in any order but when the policy is serialized to JSON/DAG-JSON/DAG-CBOR it has a canonical representation.
Then this method simply disappears, and you don't have to remember to call it in ETag for example.
There was a problem hiding this comment.
Done in 491d329: type StatementSet []Statement, any order in memory, written deduplicated and in CID order by its MarshalJSON, MarshalDagJSON and MarshalCBOR. Normalize is gone; ETag is the CID of the document as encoded, and Hilt's Decode no longer sorts.
One wrinkle: cbor-gen and dag-json-gen pick a struct field's encoder by reflect kind and inline a slice, so a generated PolicyDocument would have bypassed the set's methods. PolicyDocument (two fields) therefore has a short hand-written codec that delegates to the set, for the same reason s3.PermissionSet has one. It writes the layout the generators did (CBOR keys shortest first, DAG-JSON keys sorted); a golden test holds the bytes captured from the generated code, and the pinned ETag hashes are unchanged.
There was a problem hiding this comment.
Alternatively you can avoid the sidestep by defining as type StatementSet struct { Members []Statement }.
|
|
||
| // StatementCID returns the CID of st's DAG-JSON encoding: its sets as maps | ||
| // with sorted keys, so nothing about how the caller built them reaches it. | ||
| func StatementCID(st Statement) cid.Cid { |
There was a problem hiding this comment.
Why aren't we using this from ETag?
There was a problem hiding this comment.
Etag is the CID of the full bucket policy document and not just its statements (it includes the Version field).
There was a problem hiding this comment.
Both now go through one cidOf(marshaler) helper (491d329): ETag(d) is cidOf(&d) quoted, StatementCID(st) is cidOf(&st).
… the wire The document type is PolicyDocument (the binding keeps the command's name, Policy). Its statements are a StatementSet: any order in memory, written deduplicated and ordered by statement CID in JSON, DAG-JSON and CBOR, so Normalize is gone and ETag is the CID of the document as encoded. The generators inline a slice field by kind, so the two-field document carries a hand-written codec that delegates to the set, laid out as the generated code was (a golden test holds the bytes). ETag and StatementCID share one CID helper. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Serves S3 `GetBucketPolicy`, `PutBucketPolicy` and `DeleteBucketPolicy` as one Hilt command, `/s3/bucket/policy`, reached by service keys only. The PUT body must match the signed payload hash, and `If-Match` / `If-None-Match` are optional signed preconditions. - `?policy` classified into three operations with their own permissions - `pkg/rpc/service/bucket/policy.go`: operation, body hash check, preconditions - Handler, wiring and failure mapping; `client.Policy` for Ingot - `sigv4.PayloadHash` and a test presign option - The operation acts on the authorized bucket's DID; an empty precondition is invalid - Deployments must delegate `/s3/bucket/policy` from Hilt to Ingot (fil-forge/smelt#65 does it for local stacks) - A principal's policy list carries no per-item ETag; a caller reads it from `GetBucketPolicy`'s `ETag` header - A bucket deleted between authorization and a policy write answers `UnknownBucket` - Temporary pin: libforge to fil-forge/libforge#80; re-pin before merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…t/policy
GET, PUT and DELETE on /{bucket}?policy are answered ahead of the S3
route table: the signed request and, on a PUT, the body are forwarded to
Hilt as /s3/bucket/policy, which authenticates and authorizes them. The
routes sit ahead of versitygw's controllers because its PutBucketPolicy
validates the body as an AWS-shaped document, which a Forge policy is
not, and its backend seam carries neither preconditions nor an ETag. The
bucket authority maps Hilt's policy rejections; the routes render them
as NoSuchBucketPolicy, MalformedPolicy, PreconditionFailed,
InvalidRequest and OperationAborted, and a CreateBucket's refused
x-bucket-policy header now renders as MalformedPolicy too, the RFC's one
mapping. The in-memory store answers NoSuchBucketPolicy and NotImplemented.
Temporary pins to hilt branch srdjan/feat/iam-policy-command
(fil-forge/hilt#89) and libforge branch
srdjan/feat/s3-bucket-policy-command (fil-forge/libforge#80); re-pin
before merge.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lers (#194) Serves `GET`, `PUT` and `DELETE /{bucket}?policy` through versitygw's own route table, auth and controllers. ingot's backend forwards each operation to Hilt's `/s3/bucket/policy`. The policy document is now an AWS bucket policy (fil-forge/hilt#96), so versitygw's `PutBucketPolicy` validator accepts it and rejects anything that isn't AWS-shaped before Hilt sees it. Hilt's stricter checks still decide. - `s3frontend/policy.go`: `PutBucketPolicy`, `GetBucketPolicy` and `DeleteBucketPolicy` forward the signed request from `reqscope`. `GetBucketPolicy` forwards only a signed `GET ?policy`. versitygw's own access checks get `NoSuchBucketPolicy`, so they never reach Hilt or evaluate a policy, and an anonymous request is `AccessDenied`. - `server.go`: a `policyHeaders` middleware sets the `ETag` Hilt returned and `application/json` on a `GET`, because the backend interface returns no headers. Preconditions reach Hilt in the signed request. - `iam`: `GetUserAccount` resolves `arn:aws:iam:::user/<id>`, so versitygw's validator accepts ARN principals. Hilt checks that the principal exists. - `policy_routes.go` and its test are deleted. - `itest/forge_bucket_policy_test.go` sends AWS-shaped documents and covers a bare-name principal (refused by versitygw) and a missing `Version` (refused by Hilt). A policy request now reaches Hilt twice: versitygw's auth calls `/s3/request/authorize`, then the backend calls `/s3/bucket/policy`. ## Change log - `chore`: temporary pin of hilt to fil-forge/hilt#96 (`b2da449`), to be re-pinned to main before merge. libforge stays pinned to fil-forge/libforge#80. - `feat(iam)`: resolve user ARNs for versitygw's policy validation. - `feat(s3frontend)`: serve the bucket policy operations through the backend. - `refactor(s3)`: serve `?policy` through versitygw's controllers; delete the route intercept. - `test(itest)`: drive the bucket policy scenario with AWS-shaped documents. e2e: the `policies-local` suite passed against a fresh smelt running hilt#96 and ingot#194, with the console from fil-one/fil-one#812 (102 of 103 passed; PU3 was flaky on its first attempt while the fresh Owner org was still unnamed). ## References - fil-one/RFC#30, the tenant IAM RFC; fil-one/RFC#34 updates it to the AWS document. - fil-one/fil-one#696, the bucket access ADR. - fil-forge/hilt#96, the AWS-shaped policy document in Hilt. - fil-forge/libforge#80. - #178, the PR below this one in the stack. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Adds `/s3/bucket/policy` to the Hilt-to-Ingot delegation smelt generates, so Ingot can invoke Hilt's new policy command. Existing stacks need `./generated/generate-proofs.sh --force` and a restart. - `pkg/stack/proofs.go` and `generated/generate-proofs.sh`: the new command - `go mod tidy` - Temporary pin: libforge to fil-forge/libforge#80; re-pin before merge 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Adds the
/s3/bucket/policycommand Ingot invokes on Hilt for the S3 bucket policy operations, per the tenant IAM RFC.commands/s3/bucket:PolicybindingPolicyArguments{Request, Body},PolicyOK{ETag, Policy}🤖 Generated with Claude Code