Repository navigation
rfc: forge S3 tenant IAM - #30
Merged
Merged
Conversation
Extends the tenant management RFC with principals, bucket policies, and principal-bound access keys, implementing the fil-one bucket policies ADR (IAM M2) on Hilt and Ingot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The only caller was the one-off migration sweep, which loops over the idempotent single-principal PUT instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cache consistency, revocation recovery, concurrency, and API contract gaps must be resolved before implementation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Extends Forge tenant management with principal-based IAM enforced by Hilt and Ingot.
Changes:
- Adds principals, service credentials, and principal-bound access keys.
- Defines bucket policies, authorization, and cache invalidation.
- Specifies schema, migration, and example workflows.
File summaries
| File | Description |
|---|---|
rfcs/2026-09-forge-s3-tenant-iam.md |
Defines the proposed Forge S3 tenant IAM architecture and APIs. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 11
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A principal is a (tenant, userId) row with no key material; per-request delegations are signed with the tenant key. Narrowings reach the gateway through a new Swarf /principal/invalidate command and firehose event, published by Hilt's service identity from a configured publisher list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cache invalidation races and incomplete cross-gateway bucket deletion can preserve revoked access.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
rfcs/2026-09-forge-s3-tenant-iam.md:152
- Including
s3:ListAllMyBucketsineffective(p, b)means the set is never empty, contradicting both the omitted-empty-buckets rule and authorization step 6. An existing but inaccessible bucket would consequently take the 403 path instead ofUnknownBucket/404. Keep this tenant-level action outside the per-bucket effective set.
plus `s3:ListAllMyBuckets` (see [action vocabulary](#action-vocabulary)). A bucket with no policy has an empty effective set for every principal. The service credential is not a principal and is not evaluated against policies.
rfcs/2026-09-forge-s3-tenant-iam.md:66
- Both minting paths return the only copy of the secret once, but retries return either a credential-less 200 or 409, and there is no delete/rotation route. If Hilt commits and the response is lost, the console cannot recover the credential and migration or tenant setup is permanently stuck. Define an idempotency/replay mechanism or a reset/rotation operation before relying on this flow.
- MUST be returned once, in the body of the call that minted it, as `serviceCredential: { accessKeyId, secretAccessKey }`.
- MAY be minted for a tenant that has none through `POST /tenants/{tenantId}/service-credential`, which returns it the same way and answers 409 when one exists. This is the migration path for existing tenants.
rfcs/2026-09-forge-s3-tenant-iam.md:141
- This item is under “reject with 422” but says the same condition is reported as 404, leaving the API contract contradictory. Keep document-validation failures in the list and state the cross-tenant/not-found bucket response separately.
- a bucket that belongs to another tenant, reported as 404.
rfcs/2026-09-forge-s3-tenant-iam.md:502
- In PostgreSQL,
UNIQUE (bucket_id, principal)permits multiple rows whereprincipal IS NULL, so it does not enforce one wildcard index entry per bucket as intended. UseNULLS NOT DISTINCTon supported PostgreSQL versions, or separate partial unique indexes for explicit and wildcard principals.
UNIQUE (bucket_id, principal)
- Files reviewed: 1/1 changed files
- Comments generated: 4
- Review effort level: Balanced
Member
Author
|
@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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfce0390e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
…locked publish Service credentials become a list per tenant with mint, list, and delete routes, so a lost mint response or a rotation is a second mint and a delete. A policy write publishes an invalidation for every principal whose effective set changed in either direction, so a widening reaches a warm gateway cache. The write publishes inside its transaction with the policy, key, or principal row locked, and the authorize path reads those rows with a shared lock, so a gateway cannot refill its cache from the old policy between publish and commit. The Tenant API bucket create is dropped: a bucket without a policy is reachable by service credentials only, and the console writes the policy right after creating the bucket over S3. With it goes the Ingot requirement to register buckets learned from bucket-info. Bucket deletion states the one-gateway assumption it relies on. ListAllMyBuckets leaves the per-bucket effective set, the cross-tenant bucket case is a 404 on its own, the two bucket-configuration reads are authorized at Hilt on every request, and the wildcard index row gets a partial unique index. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The invalidation invocation names Hilt as its own subject, the shape the revoke command already uses, because the standard validator refuses a foreign subject with no proofs. Existing keys are removed by `hilt migrate iam` before the schema migration, which now refuses to run while a key remains. A service credential stores its accessKeyId as its name so the schema is unchanged. Principal removal holds the principal row lock across idempotent cleanup steps instead of claiming one transaction over tables and the vault. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 10, 2026
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 2, 2026
Bucket policies are read and written over S3 (GetBucketPolicy, PutBucketPolicy, DeleteBucketPolicy) with a service key holding the matching permission, per fil-one/RFC#30, so the policy routes, their precondition parameters, the ETag header and the 412 response go. The per-principal key list goes too; the tenant key list takes a principalId filter. The permission enum gains the three policy actions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 2, 2026
The PolicyAction description still excluded only CreateBucket, DeleteBucket and ListAllMyBuckets, so after GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy joined AccessKeyPermission it read as if s3:* covered them. The enum, the AccessKeyPermission text and fil-one/RFC#30 all say a policy never grants them; the prose now agrees. Regenerated the client: only the JSDoc changed. The new type test pins the enum the description talks about. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 2, 2026
The storage system serves bucket policies as the S3 operations GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy (fil-one/RFC#30, fil-forge/hilt#89), so the iam arm's policy methods sign those with the tenant's console key instead of calling management-API routes. The preconditions ride as signed If-Match / If-None-Match headers and the ETag comes back in a header, both through command middleware. The S3 error codes map onto the policy errors the routes and the fanout already handle; the console key gains the three policy permissions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The console creates a bucket with CreateBucket and then PutBucketPolicy with If-None-Match: *; the x-bucket-policy header moves to alternatives. Policy documents use AWS capitalization: Statement, Sid, Effect, Principal, Action, Allow, Deny. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-forge/ingot
that referenced
this pull request
Oct 5, 2026
…header fil-one/RFC#30 now names the bucket policy fields Statement, Sid, Effect, Principal and Action with effects Allow and Deny, and CreateBucket no longer carries an x-bucket-policy header, so Hilt's create never answers InvalidBucketPolicy. The test documents use the new names, and the create-path MalformedPolicy mapping goes; the policy routes still render a refused PutBucketPolicy body as MalformedPolicy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 5, 2026
Open
The ETag hashes statements, principals and actions as DAG-JSON maps, so order and duplicates do not change it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 5, 2026
* docs(adr): bucket access by region (IAM M2, FIL-1017) FIL-1017 asks that an Owner or Admin can limit a member to a subset of the org's buckets, and that the member sees and acts on that set alone. Aurora and FTH cannot model it. Forge can. So the console carries two access models and branches on the region in hand. Forge gets AWS-shaped bucket policies: one policy per bucket, held by the service orchestrator, with Allow and Deny statements naming individual members. Aurora and FTH keep the scoped keys they ship today, capped by the member's role. The storage system never learns a FilOne role; the role decides who may edit a policy and what may go into a statement. Supersedes #642, #661, #662 and #667, and targets main directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(adr): reconcile with the Forge S3 tenant IAM RFC The Forge-side contract now exists as the Forge S3 tenant IAM RFC, and it settles things this ADR asked for or guessed at. Five claims here described the parent RFC's mechanism rather than the one the design adopts. - A narrowing reaches the gateway over the revocation firehose, not by waiting for a cached chain to expire, so the midnight bound is gone. - Deleting a bucket revokes nothing: no delegation names a single bucket, and the gateway refuses buckets it does not know. - Bucket create takes its policy in the same call and commits both together, so the two-step write and its retry are gone. - Hilt enforces no key limit, so per-member credentials spend nothing there. - The flip retires the console's own credential too, and the gateway ships before the management API. Compare-and-set is specified as an ETag, so it moves from an ask to the stated mechanism. Bucket enumeration is named as the one action a Deny cannot remove, and the effective-action set the gateway must hold is named as what makes Deny enforceable at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(adr): drop groups from future work A group is not a valid principal in an AWS resource-based policy, bucket policies included: the Principal element takes accounts, users, roles, role sessions, federated users, services, or everyone. Listing groups as future work pointed away from the S3 shape this design is matching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Update reference to enforceability memo in documentation Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * docs(adr): tighten bucket-policies prose and assume one region per network Rewrite the M2 bucket access ADR for readability: move the production impact and the term definitions up front, turn label-style leads into plain sentences, split the densest paragraphs, and rewrap to 80 columns. State that one Forge network serves one region, so a tenant id is unique per region, and drop the region-qualified external id requirement. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): key list carries principal only; console writes the policy after bucket create The access-key list returns each key's principal and no per-key access, which the console reads once per principal. Every change to a principal's access publishes before Hilt acknowledges it, widenings included. Bucket creation is two console calls, S3 create then the policy write, and a bucket without a policy is reachable by the tenant-wide credential only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): the console sends a bucket's first policy in x-bucket-policy The tenant IAM RFC carries a new bucket's policy on the S3 create request, so the storage system writes the bucket and its policy together and no bucket outlives a failed policy write. The flow here said the console wrote the policy in a second call after the create; it now matches the RFC and the implementation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): a key's grant is materialized as policy-derived delegations A policy edit rewrites the delegations of every key bound to a changed principal over the edited bucket, so the principal needs no identity of its own for a revocation to target, and "touches no key" no longer describes the mechanism. Matches RFC fil-one/RFC#30. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): name the labels the console writes its default statements under Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(adr): the console keeps one credential per tenant The console signs member traffic with the tenant credential and refuses a scoped member's request itself from the resolved per-bucket access. The per-member console credential moves to the rejected options, priced by the secret store, request-path mint, cache and removal work it needs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): an Owner or Admin mints service keys from the console The orchestrator mints either key kind through one method, the shape of the request deciding. A service key carries its own permissions and bucket list, answers to no policy, and takes keys.create_service. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): format the bucket policies ADR with oxfmt Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(adr): align the bucket policies ADR with the Forge S3 tenant IAM RFC Policy documents use AWS's field names (Statement, Sid, Effect, Principal, Action). The console writes a bucket's first policy with PutBucketPolicy after CreateBucket and finishes it from a durable job on failure. Policies are read and written over S3 with the service key. The console's key is an ordinary service key, listed and counted like any other, and every existing key stays in service through the network flip. Deleting a bucket revokes the delegations over it. The key list carries a type per key, the policy actions join the excluded set, and the batched principal write is dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(adr): match the review threads on Owner denies, retention grants and removal A Deny naming an Owner or Admin limits their bound keys; the console still serves them. An Admin's edit keeps the retention grants an Owner wrote. Removing a principal takes it out of each statement and drops a statement only when it names nobody. The Allow \ Deny code span stays on one line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
Forge regions gain IAM-style bucket policies (fil-one/RFC#30, ADR #696). This lands the shared vocabulary everything else builds on: the policy document schema in the shape Hilt stores, the 14-action vocabulary with labels and groups, effective-action evaluation, the roster statements the console writes for Owners and Admins, the `buckets.policy_manage` permission (Owner, Admin), three policy error codes, the three `bucket_policy.*` audit events, and the principal-bound key fields. A principal-bound key carries no permission set of its own, so `canRetainAccessKey` keeps one under any role that can mint and revokes it only on demotion to a role that cannot. No region serves the `iam` access model yet; nothing here changes behavior. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
…contract The management API contract gains the `iam` capability set from fil-one/RFC#30: principals under `/tenants/{tenantId}/principals`, one policy per bucket under `/tenants/{tenantId}/buckets/{bucketName}/policy` with ETag compare-and-set, and a principal-bound shape for access key creation. The action enum gains the two multipart actions, so the console key and scoped keys now carry those. Bucket-configuration reads classify as ListBucket, so the enum carries no action for them and the console key keeps dropping those permissions with a warning. Regenerated `@filone/orchestrator-client` from the contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
Bucket policies are read and written over S3 (GetBucketPolicy, PutBucketPolicy, DeleteBucketPolicy) with a service key holding the matching permission, per fil-one/RFC#30, so the policy routes, their precondition parameters, the ETag header and the 412 response go. The per-principal key list goes too; the tenant key list takes a principalId filter. The permission enum gains the three policy actions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
The PolicyAction description still excluded only CreateBucket, DeleteBucket and ListAllMyBuckets, so after GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy joined AccessKeyPermission it read as if s3:* covered them. The enum, the AccessKeyPermission text and fil-one/RFC#30 all say a policy never grants them; the prose now agrees. Regenerated the client: only the JSDoc changed. The new type test pins the enum the description talks about. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
Per fil-one/RFC#30, the access-key responses carry a required `type`, `service` or `principal`, so the console reads the key kind from the response instead of guessing it from which fields are present. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
The storage system serves bucket policies as the S3 operations GetBucketPolicy, PutBucketPolicy and DeleteBucketPolicy (fil-one/RFC#30, fil-forge/hilt#89), so the iam arm's policy methods sign those with the tenant's console key instead of calling management-API routes. The preconditions ride as signed If-Match / If-None-Match headers and the ETag comes back in a header, both through command middleware. The S3 error codes map onto the policy errors the routes and the fanout already handle; the console key gains the three policy permissions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-one/fil-one
that referenced
this pull request
Oct 7, 2026
The management API now states each key's `type`, so issueAccessKey returns a principalId only for a key typed `principal` instead of for any response that happens to carry a `principal` field (fil-one/RFC#30). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyropy
added a commit
to fil-forge/ingot
that referenced
this pull request
Oct 9, 2026
fil-one/RFC#30 adds two Hilt failures Ingot has to render. An authorize that waited out Hilt's lock timeout behind a policy write answers TemporarilyUnavailable, which becomes ServiceUnavailable (503) so the client retries. A CreateBucket whose x-bucket-policy header is unsigned or fails validation answers InvalidBucketPolicy, which becomes InvalidArgument (400) naming the header. The names are matched literally: the pinned hilt predates the constants. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pyropy
added a commit
to fil-forge/ingot
that referenced
this pull request
Oct 9, 2026
…header fil-one/RFC#30 now names the bucket policy fields Statement, Sid, Effect, Principal and Action with effects Allow and Deny, and CreateBucket no longer carries an x-bucket-policy header, so Hilt's create never answers InvalidBucketPolicy. The test documents use the new names, and the create-path MalformedPolicy mapping goes; the policy routes still render a refused PutBucketPolicy body as MalformedPolicy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyropy
added a commit
to fil-forge/swarf
that referenced
this pull request
Oct 9, 2026
## Summary RFC 30 (fil-one/RFC#30) has Hilt rotate a principal-bound key's delegations on every policy change and send every revocation produced by one write to Swarf in a single request, so a write waits on one round trip however many keys it touches. The client gains `PublishBatch`, which builds one self-signed `/ucan/revoke` invocation per revoked delegation and sends them in one container through ucantone's `ExecuteBatch`. Every receipt is checked; the first refused one fails the call. The service needs no change: a ucantone server already executes every invocation in a request container. ## Change log - `pkg/client`: `PublishBatch(ctx, revoker, revoked []ucan.Delegation)`; the executor is the ucantone HTTP client so both `Execute` and `ExecuteBatch` are available. - `go.mod`: ucantone bumped to `b59f1d1fcbb6` for `execution/batch`. - Tests: `TestPublishBatch` (one request for two delegations, an empty batch publishes nothing, a refused revocation fails the call). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the tenant management RFC with principals, bucket policies, and principal-bound access keys, implementing the fil-one bucket policies ADR (proposed in fil-one/fil-one#696) on Hilt and Ingot.
The Fil One console keeps one service key per tenant, signs all member traffic with it, and enforces a scoped member's access itself from the principal's effective actions. Members mint their own principal-bound keys; an Owner or Admin can also create a service key through the console, authorized from its own permissions and buckets as the parent RFC specifies.
📖 Preview