Skip to content

feat(iam): refuse to cache delegations seen revoked - #177

Open
pyropy wants to merge 7 commits into
mainfrom
srdjan/feat/iam-revoked-set
Open

feat(iam): refuse to cache delegations seen revoked#177
pyropy wants to merge 7 commits into
mainfrom
srdjan/feat/iam-revoked-set

Conversation

@pyropy

@pyropy pyropy commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Refuses to cache an authorize response that carries a delegation Ingot has seen revoked, so a Hilt write whose commit failed after publishing cannot leave stale grants cached until midnight. Such a request is authorized once and not cached.

  • KeyProofs remembers revoked CIDs until the next UTC midnight
  • The uncached authorize path with a request-scoped delegation store
  • adminAccount helper
  • Tests for the recorded set and the uncached response

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 22, 2026 17:13
Copilot AI reviewed Sep 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pyropy
pyropy added this pull request to stack #179 September 22, 2026 17:13
@pyropy

pyropy commented Sep 23, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T14:11:23.444742Z eba04d0 Manual request
ℹ️ 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" or "@codex security review".

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]
chatgpt-codex-connector Bot reviewed Sep 23, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e9aeafd4f

ℹ️ 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".

Comment thread iam/service.go Outdated
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 3e9aeaf to e7ccdc6 Compare September 28, 2026 13:45
@pyropy

pyropy commented Sep 28, 2026

Copy link
Copy Markdown
Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Sep 28, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a8a4a9d81a

ℹ️ 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".

Comment thread iam/keyproofs.go
@pyropy
pyropy marked this pull request as draft September 28, 2026 17:05
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch 3 times, most recently from a8a4a9d to 7591ff9 Compare October 1, 2026 12:11
@pyropy

pyropy commented Oct 1, 2026

Copy link
Copy Markdown
Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 1, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7591ff9e8e

ℹ️ 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".

Comment thread iam/service.go Outdated
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch 3 times, most recently from ddff603 to eba04d0 Compare October 1, 2026 14:06
@pyropy

pyropy commented Oct 1, 2026

Copy link
Copy Markdown
Author

@codex review

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 1, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eba04d0ab9

ℹ️ 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".

Comment thread iam/keyproofs.go Outdated
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 696bc5f to eb9160d Compare October 1, 2026 15:18
@pyropy
pyropy marked this pull request as ready for review October 2, 2026 11:02
@pyropy
pyropy requested a review from a team October 2, 2026 11:03
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch from eb9160d to 8977533 Compare October 2, 2026 11:19
pyropy and others added 6 commits October 2, 2026 14:15
@pyropy @claude
feat(iam): refuse to cache delegations seen revoked
4b5bb9c 
A Hilt write publishes its revocations before it commits. When Swarf
accepted them and the commit then failed, the revoked delegations stayed
stored at Hilt, so Ingot dropped the key's cache on the revocation and
refilled it from Hilt's next response, which carried the same delegations.
The console's retry republished identical revoke invocations, which Swarf
deduplicated, so the intended change waited for midnight.

KeyProofs now remembers every revoked CID until the next UTC midnight plus
clock skew, the horizon of everything cached from an authorize response,
whether or not a store held the delegation. A response carrying a
remembered CID authorizes the one request it answers and caches nothing:
no signing key, tenant, action set or delegations. The request keeps a
store of its own holding the response's delegations, so its onward
retrieval still has a chain. The key returns to Hilt on every request
until a committed write issues fresh delegations, whose random nonces give
them new CIDs. A restart clears the set with the caches.

Implements the error-recovery section of fil-one/RFC#30 (d2cd4a6).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@pyropy @claude
fix(iam): name the admin-account helper past main's account variable
970d13c 
main's GetUserAccountForRequest now holds the local fast path's result in
a variable named account, which shadowed the helper of the same name.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@pyropy @claude
fix(iam): check for a revocation and cache the delegations in one step
478c450 
The request path checked the revoked set and then added the delegations to
the key's store as two separate steps. A revocation landing between them
found the store without the CID, cleared nothing, and left the revoked
delegation cached until the horizon. KeyProofs.Revoked is now
DepositUnlessRevoked: the check and the add hold the same lock, so a
revocation lands either before (the check sees it) or after (the store
holds the CID and is dropped). A race test runs the two against each other.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@pyropy @claude
fix(iam): refuse to cache a bucket-info delegation seen revoked
cb14d43 
A leaf-only authorize response is completed from /s3/bucket/info, and those
delegations went into the key's store with no revocation check: a revoked
intermediate there was cached along with the signing key, tenant and action
set. cacheProofs now deposits through DepositUnlessRevoked on the shared
store and reports a refusal, which takes the existing uncached path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pyropy @claude
fix(iam): remember a revoked CID until restart, not midnight
6dca938 
A revocation names only the CID, and the delegation behind it may never
expire (/s3/bucket/info chains carry no expiry and DelegationCache keeps
them until restart). Forgetting the revocation at midnight let the next
authorize response pair a fresh leaf with the same stale bucket-info
delegation and repopulate the caches. The revoked set now keeps entries
for the process lifetime.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pyropy @claude
fix(iam): bound the revoked set by a TTL renewed on each sighting
81c84b2 
Remembering every revoked CID until restart grew without bound. An entry
only matters while Hilt still serves the revoked grant (a write whose
commit failed), so it now lives revokedTTL (48h) and each refusal of a
response carrying it renews it. A failed write whose grant goes unserved
for longer than that is forgotten; that gap is noted in the code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pyropy @claude
chore(iam): drop the ponytail: prefix from two comments
e9edb8c 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pyropy
pyropy force-pushed the srdjan/feat/iam-revoked-set branch from 8977533 to e9edb8c Compare October 2, 2026 12:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pyropy