Conversation
A co-tenant block in the shared opencode-handles.json is only unreachable by our validator because readCustodyHandles filters providers by provider/serve before parsing accounts. Nothing pinned that ordering, so a refactor that validated accounts first would silently let a foreign block reach our validator. Add reader tests: a foreign xai block carrying an unknown account key (minTtlMs) beside our block reads as ready with our accounts and no corruption, and hostile foreign entries (non-array accounts, null, a number, a provider-only object) are ignored. Add a writer round-trip test that verifies the write landed before asserting the foreign block's unknown key survives byte-identically.
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.
Tests only. No production change — this pins behaviour that is already correct but currently unasserted.
Why
opencode-handles.jsonis a shared, co-tenant manifest. Blocks are keyed byprovider+serve; ours isanthropic/anthropic-auth. Sibling plugins own their own blocks and evolve them independently.A sibling tenant is adding an optional
minTtlMskey to the account objects in their block and asked us to confirm we tolerate it. We do — structurally:readCustodyHandles(packages/core/src/claustrum.ts:482) filtersjson.providerstoentry.provider === providerat:501-506before any account parsing, then selects onentry.serve === serveat:508. Foreign blocks never reach the account loop.writeCustodyHandleManifestEntryLocked:1424passes foreign blocks through by identity (providers.map((p, i) => i === blockIndex ? {…} : p)), never re-serialising them from a parsed model.Neither property was pinned by a test. Both are consequences of current statement order and object-spread choices. A refactor that validated accounts before filtering by provider, or rebuilt the providers array instead of mapping it, would silently make a foreign tenant's block reachable — or drop it — and nobody would find out until a live shared manifest took someone dark.
The tests
reads our accounts while ignoring a foreign block with unknown account keys— a foreign block carryingminTtlMsbeside ours; our read returns our accounts withcorruptLabelsempty.reads our accounts while ignoring hostile foreign provider entries— foreignaccountsthat is not an array, plusnull,42, and a{provider}-only object as siblings inproviders[]; our read still succeeds.preserves a foreign block unknown key byte-identically when writing our account— writes a new account into our block and asserts the foreign block is serialise-identical afterwards.Verification
Both mutations break the isolation, not the tests:
reads our accounts while ignoring a foreign block with unknown account keys—Expected -7 / Received +1preserves a foreign block unknown key byte-identically when writing our account—Expected -6 / Received +6My first attempt at the reader mutation was too weak and passed. Removing only the provider filter leaves the
serveselector at:508, which still lands on our block — a no-op for this fixture. That was my mutation being wrong, not the test being vacuous; removing both filters reddens it. Worth stating because a mutation that fails to redden looks identical to a test that does not work, and the difference is which one you fix.The write-status assertion in the third test is deliberately first, before any preservation assertion.
writeCustodyHandleManifestEntryreturns{status:'refused', reason}rather than throwing, so a round-trip test that skips that check reads the untouched original file and reports preservation that never happened. That is exactly how my initial hand-probe of this produced a false positive.Gates: core 199/0 · opencode 1897/0 (1894 + 3) · typecheck clean.
Known limit, stated rather than implied
Our preservation is by construction, not by assertion — there is no post-mutate check that foreign blocks are unchanged. The sibling tenant's writer asserts it inside their locked write, so their regressions fail at the writer; ours would fail only at test time, once these tests exist. Converging on a writer-level assertion is the better end state; this PR closes the gap that is cheap to close now.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds regression tests for cross-tenant manifest isolation in the shared
opencode-handles.json. No production change; these assertions pin behavior that a refactor could otherwise silently break.Tests
minTtlMskey and returns only our accounts with no corruption.null, a number, and a provider-only object.Known limit
Written for commit 061657d. Summary will update on new commits.