Protect _config from ownership hijack, deletion, and query leakage (#67)#100
Merged
Conversation
…y leakage (#67) Nothing guarded the stack's identity record: update('_config', { entityId }) silently re-anchored ownership out from under every running permission check, delete('_config') (soft or hard) left the stack unreadable or unopenable, and query exclusion was a hardcoded WHERE clause one adapter happened to remember. Per the issue's layering principle, these are integrity guards that belong in Stack (inherited by every adapter and by ScopedStack, which delegates) rather than adapter-specific conventions — query exclusion is the one exception, since it has to stay a WHERE predicate to avoid breaking pagination, and already lives in sqlite-shared for both real SQL adapters. - Stack.update('_config', ...) rejects a changed entityId (StackConflictError); other fields (timezone) update normally. - Stack.delete('_config') is rejected unconditionally, soft or hard. - Stack.restoreVersion('_config', ...) rejects restoring a snapshot whose entityId disagrees with the live record's. - MemoryAdapter now excludes _config from generic queries too, matching the real adapters and making the exclusion rule testable/enforceable at the core level. docs/spec.md: _config protections stated under Stack initialization, the query-exclusion rule made normative, and the 409 error-table row extended. Fixes #67 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KDmUW3oNxPSCLf2ftqaF8q
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
The
_configsingleton — the stack's identity, read at open and consulted by every permission check — was unprotected:update('_config', { entityId: 'someone-else' })passed schema validation cleanly and silently re-anchored stack ownership.delete('_config'), soft or hard, left the stack unreadable/unopenable.WHEREclause convention that a future adapter could easily forget.Per the issue's layering principle — adapters are storage engines,
Stackis the library's invariant layer,ScopedStackis per-requester policy — the fix for (1) and (2) belongs inStackitself, inherited by every adapter and byScopedStack(which delegates toStack) without any adapter-specific code. Query exclusion is the deliberate exception: it has to stay aWHEREpredicate to avoid post-filtering breaking pagination, and turns out to already be centralized insqlite-sharedfor both real SQL adapters (landed as part of #46's refactor since this issue was filed).Changes
Stack.update('_config', ...)rejects a changedentityIdwithStackConflictError(409) — a stack-integrity conflict, not a schema violation, since the new value is a perfectly valid string. Other fields (timezone) update normally.Stack.delete('_config')is rejected unconditionally, soft or hard, withStackConflictError.Stack.restoreVersion('_config', ...)rejects restoring a snapshot whoseentityIddisagrees with the live record's — closes the same door via the restore path.MemoryAdapter(core's test double) now excludes_configfrom generic queries too, matching the real adapters' behavior and making the exclusion rule testable at the core level (previously it didn't model_configas a record at all).docs/spec.md:_configprotections documented under Stack initialization, the query-exclusion rule stated as normative, and the 409 error-table row extended to mention it.Since
ScopedStack.update()/delete()/restoreVersion()all delegate to the correspondingStackmethod, these guards apply identically whether called directly or through aScopedStackview — including for the stack owner.Cross-referenced prerequisites (#49 identity, #53 error codes, #46 shared SQL layer, #50 MemoryAdapter fidelity, #52 fixtures, #55 ID validation, #62 restore validation) were already closed before this landed; #49 (DID identity) remains open but isn't a blocker — this issue only guards the field that exists today.
Test plan
packages/coretypecheck cleanpackages/corelint cleanpackages/coretest suite passing (430/430), including 11 new cases:update()rejects anentityIdchange, allows atimezonechange, and treats settingentityIdto its current value as a no-oprestoreVersion()rejects a snapshot with a differententityId, allows one with the same_config;get('_config')still worksScopedStack-as-owner delegation: the owner can't bypass either guard through the scoped surfacerecord-adapter-sqlite(88/88) andsqlite-shared(36/36) suites unaffected — confirmed no other package manipulates_configthroughStack(only via direct SQL/adapter-level test setup, which this change doesn't touch)Generated by Claude Code