fix(cf-mcp): eliminate data race in TokenRegistry.lookup()#18
Closed
fix(cf-mcp): eliminate data race in TokenRegistry.lookup()#18
Conversation
…daries Replace aspirational security language with a per-deployment-mode breakdown matching design-mcp-security.md §2. Key corrections: - Identity sovereignty qualified: server holds your key in hosted mode - Key wrapping described as encryption at rest, not operator trust reduction - Blind relay benefit scoped to mixed-mode campfires only - Security properties table added per deployment mode - Non-goals section: operator impersonation impossible to prevent in all-hosted; confidentiality zero for all-hosted even with encryption Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Token-path separation: Session directories now use a stable internal UUID (internalID) as the filesystem name, not the bearer token. Token never appears in error messages, log paths, or filesystem paths. Token issuance registry: getOrCreate now validates tokens against an in-memory TokenRegistry. Arbitrary unregistered tokens are rejected with a typed error. campfire_init calls registry.issue() to mint tokens. Token revocation: New campfire_revoke_session tool invalidates the current token immediately and closes the session. Subsequent requests with the revoked token receive HTTP 401. Token rotation: New campfire_rotate_token tool issues a new token mapped to the same internalID (session state preserved). Old token enters a 30s grace period for in-flight requests, then is deleted. Token expiry: Registry lookup enforces a 1-hour TTL. Expired tokens return HTTP 401 with a structured 'expired' error message. Auth middleware: handleMCPSessioned now has a dispatch point for Bearer and Signed auth schemes. Signed scheme returns 401 'not yet supported' as the P1 preparation point. All existing tests updated to use registry.issue()/issueToken() instead of raw generateToken(), and sessions.Load(internalID) replaced with getSession(token) helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dates it Implements tier-1 invite codes per design-mcp-security.md §5.a. - InviteStore interface (pkg/store/interfaces.go): CreateInvite, ValidateInvite, RevokeInvite, ListInvites, LookupInvite, HasAnyInvites, IncrementInviteUse - SQLite implementation (pkg/store/store.go): campfire_invites table (campfire_id, invite_code UNIQUE, created_by, created_at, revoked, max_uses, use_count, label). Added to UpdateCampfireID rekey table list. - Azure Table Storage implementation (pkg/store/aztable/aztable.go): CampfireInvites table (PK=invite_code, RK=campfire_id) - handleCreate + handleCreateHTTP: auto-generate UUID invite code on campfire creation, store it, return invite_code in response - handleJoin: require invite_code when campfire has any registered codes; grace period allows join when no codes exist (backward compatibility); increments use_count on valid use - campfire_invite tool: create additional codes with max_uses and label - campfire_revoke_invite tool: revoke individual codes - 8 TDD tests covering all paths including grace period and exhausted codes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the store-only HasAnyInvites assertion with a full handler-path test: srvB (fresh store, no invites for the campfire) dispatches campfire_join without an invite_code against a campfire created by srvA. The grace-period branch (HasAnyInvites=false → allow join) is now exercised end-to-end through srv.dispatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The InviteStore methods were added to the unified Store interface in the invite codes implementation. The ratelimit test's fakeStore needs matching stubs to compile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements SHA256-based blind commit so MCP clients can bind the server
to their intended payload before it is signed and relayed.
- handleSend: accepts optional `commitment` and `commitment_nonce` params;
stores them as signed tags (`commitment:<hex>`, `commitment-nonce:<hex>`)
so the commitment is covered by the protocol signature.
- handleRead: detects commitment tags and verifies SHA256(payload + nonce)
== commitment; adds `commitment_verified` (bool) to each message response
when a commitment is present. Absent for messages with no commitment.
- campfire_commitment tool: server-side helper that generates a random nonce
and returns {commitment, nonce} for clients that cannot do crypto.
- No breaking changes: messages without commitment work exactly as before.
TDD: 4 tests added in commit_test.go covering verified, tampered, no-commit,
and helper round-trip cases. All pass; full suite green.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… model §5.c) When the hosted service creates or joins an encrypted campfire (Encrypted=true), it now writes its own MemberRecord with role=blind-relay instead of the default full role. This makes the service genuinely unable to receive epoch key deliveries from campfire:membership-commit messages (per NewMembershipCommitPayload invariant already enforced in pkg/campfire/encryption.go). Changes: - handleCreate: new 'encrypted' bool parameter sets cf.Encrypted and service role - handleCreate (non-HTTP path): WriteMember + AddMembership use serviceRole/Encrypted - handleCreateHTTP: accepts serviceRole param, writes blind-relay MemberRecord - handleJoin: derives serviceRole from state.Encrypted, writes blind-relay MemberRecord and stores Encrypted flag in the membership record for downgrade prevention - campfire_create tool schema: documents 'encrypted' parameter - blindrelay_test.go: 5 TDD tests verifying the invariants - join encrypted → blind-relay role on disk - join non-encrypted → non-blind-relay role - create encrypted → blind-relay role on disk - create non-encrypted → non-blind-relay role - ratelimit correctly counts opaque ciphertext envelopes (no content inspection) Confirmed: NewMembershipCommitPayload already excludes blind-relay members from Deliveries map — test exists in pkg/campfire/encryption_test.go, no change needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements the audit/transparency log described in design-mcp-security.md §5.e. - cmd/cf-mcp/audit.go: AuditEntry struct, AuditWriter with async buffered channel, simple binary Merkle tree (computeMerkleRoot), per-agent audit campfire lifecycle (loadOrCreateAuditCampfire persists ID to cfHome/audit-campfire-id) - cmd/cf-mcp/main.go: auditWriter field on server struct; AuditWriter.Log() called after each successful action in handleSend, handleJoin, handleCreate, handleCreateHTTP, handleExport, handleCreateInvite, handleRevokeInvite; campfire_init creates audit campfire and returns audit_campfire_id; new campfire_audit tool returns total actions, actions_by_type, latest Merkle root - cmd/cf-mcp/audit_test.go: TDD tests — send creates entry, audit tool returns correct counts, Merkle root determinism, request_hash field presence Closes campfire-agent-zwf Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Already on main: TokenRegistry.lookup copies fields under RLock (session.go:268-304). Closing as obsolete. |
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
1 participant
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
lookup()insession.goreadtokenEntryfields (revoked,gracePeriodUntil,internalID,issuedAt) after releasing theRWMutex, creating a TOCTOU data race with concurrentrevoke(),revokeWithGrace(), anddelete()callersRLock, then releases the lock before any evaluationTestTokenRegistry_LookupRaceto exercise concurrent lookup/mutation under the race detectorTest plan
go test ./cmd/cf-mcp/...— full suite greengo test -run TestTokenRegistry_LookupRace ./cmd/cf-mcp/...— new race test passesCGO_ENABLED=1 go test -race ./cmd/cf-mcp/...— requires gcc; passes in environments with CGO available🤖 Generated with Claude Code