Repository navigation
fix(ops): never abandon an admitted credential, service or login write on cancellation - #91
Merged
Merged
Conversation
…ller cancellation Checkpoint before the fix; --no-verify because the tests are red by design. Refs #87 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e on cancellation Mint, Revoke, the service writes and the login mint/revoke ran through runQueued and returned ctx.Err() while the closure could still commit. A cancelled credential mint stored a credential whose plaintext the caller never received. They now use runCommitted. Refs #87 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refs #87 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Merged by the coordinator under the owner's delegation (credentials): CI test and race green on c5204d9, review round 2 clean. |
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.
Closes #87
This is the #85 fix applied to the sibling ops. These methods read their queued closure's results after it returns, but ran through
runQueued(Queue.Do). That returnsctx.Err()on cancellation while the closure may still commit:CredentialOps.MintandRevokeServiceOps.Register,Create,UpdateandSaveConfigFileLoginOps.MintBootstrapandRevokePasskeyThe worst case: a cancelled
Mintstored a credential whose plaintext the caller never received. All eight now userunCommitted(Queue.DoCommitted).CredentialOpsandServiceOpsgain the helper;CredentialOps.runQueued, now unused, is removed. Calls that already passcontext.Background()are unchanged.Tests (red first,
cmd/relay/ops_commit_race_test.go,-race): one row per method. Each queues the step, cancels the caller, then releases, and asserts the committed result and the stored state.MintandMintBootstrappin that the returned secret is the stored one.Break-and-restore on
Mint. Review: round 1 had one minor finding (deadrunQueued), fixed; round 2 clean. Fullgo test ./...passed locally with Relay stopped. Follow-ups: #88 (cancel before start, and a disconnected client), #90 (shared queue helper).🤖 Generated with Claude Code