opencode: scope a manifest credential id to its provider segment

crates/credentials-module/src/bin/cli_support/opencode_files.rs

@@ -869,7 +869,27 @@ fn validate_handle_file(file: &HandleFile) -> Result<(), OpenCodeFilesError> {
                     account.label
                 )));
             }
-            if account.credential_id.is_empty() {
+            // Must match `parseHandleFile` in packages/client/src/handles.ts. THIS IS A
+            // WRITER: `validate_handle_file` runs from `write_handle_file_for_tenant` and
+            // `verify_handle_written`, so a rule missing here lets `ck auth` ORIGINATE a
+            // row the TypeScript reader refuses -- and that reader refuses the whole
+            // document, so one bad row written here denies every tenant in the file.
+            //
+            // Until this commit the check was emptiness only, which let `ck auth` write
+            // `oauth:openai` into an `anthropic` block: the exact cross-provider smuggle
+            // the TypeScript side was tightened to reject. Two implementations of one
+            // predicate in one repo, diverging because the fix landed on the reader.
+            //
+            // Segment 2 must BE the provider block; NO segment may be empty. Segment 1
+            // (kind) is an open set -- oauth, chatgpt, antigravity, apikey are all live --
+            // and segment 3+ (label) is operator-chosen and optional, so neither is
+            // constrained beyond non-emptiness. `:anthropic:x` and `oauth:anthropic:`
+            // satisfy the provider rule literally while naming ids that cannot exist.
+            let segments: Vec<&str> = account.credential_id.split(':').collect();
+            if account.credential_id.is_empty()
+                || segments.get(1) != Some(&provider.provider.as_str())
+                || segments.iter().any(|segment| segment.is_empty())
+            {
                 return Err(OpenCodeFilesError::Invalid(format!(
                     "provider {index} account {} has invalid credential id",
                     account.label

crates/credentials-module/tests/cli_opencode.rs

@@ -263,6 +263,66 @@ fn hostile_provider_ids_and_account_labels_are_refused_by_the_rust_handle_valida
     }
 }
 
+/// Holds the WRITE path specifically. The sibling arms parse raw fixtures and would stay
+/// green if `validate_handle_file` were dropped from `write_handle_file_for_tenant`, and
+/// that is the direction that matters: a writer without the rule ORIGINATES a row the
+/// TypeScript reader refuses wholesale, denying every tenant in the shared manifest.
+///
+/// Constructed in memory rather than parsed, because the point is that a caller already
+/// holding a `HandleFile` cannot persist an invalid one -- no deserialization step stands
+/// between this value and the disk.
+///
+/// THE WRITER VALIDATES TWICE -- once on the caller's value and once on the merged result
+/// after the tenant block is folded in -- so REMOVING EITHER ONE ALONE LEAVES THIS ARM
+/// GREEN. That was measured, not assumed: deleting only the entry check kept all 71 tests
+/// passing, and the arm reddens only when both go. So this holds the WRITE PATH as a
+/// whole and does NOT pin either call site individually; a refactor that drops one of the
+/// two will not be caught here. Stated because the alternative is a reader inferring
+/// coverage from the name, which is how the gap this arm closes was created.
+#[test]
+fn an_invalid_handle_file_is_refused_at_the_write_path() {
+    let root = tmp_root("write-path-validation");
+    let path = root.path().join("opencode-handles.json");
+
+    let invalid = opencode_files::HandleFile {
+        version: 1,
+        providers: vec![opencode_files::HandleProvider {
+            provider: "deepseek".into(),
+            shape: opencode_files::HandleShape::Api,
+            serve: "opencode-claustrum".into(),
+            accounts: vec![opencode_files::HandleAccount {
+                label: "main".into(),
+                handle: "ckh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".into(),
+                // Scoped to another provider: the smuggle the TypeScript reader rejects.
+                credential_id: "oauth:anthropic".into(),
+                superseded: Vec::new(),
+            }],
+        }],
+    };
+
+    let err = opencode_files::write_handle_file(&path, &invalid)
+        .expect_err("the write path must refuse a cross-provider credential id");
+    assert!(
+        err.to_string().contains("invalid credential id"),
+        "unexpected error: {err}"
+    );
+
+    // Refused BEFORE touching disk. A writer that validates after creating the file
+    // leaves a partial artifact for the next reader, and "it returned an error" does not
+    // distinguish the two.
+    assert!(
+        !path.exists(),
+        "a refused write must not leave a file behind"
+    );
+
+    // Positive control: the same shape with a correctly scoped id must persist, so the
+    // refusal above is the predicate acting rather than the writer refusing everything.
+    let mut valid = invalid;
+    valid.providers[0].accounts[0].credential_id = "apikey:deepseek:main".into();
+    opencode_files::write_handle_file(&path, &valid).expect("a valid file must persist");
+    assert!(path.exists(), "the valid write must produce a file");
+}
+
 #[test]
 fn handle_file_debug_redacts_live_and_superseded_capabilities() {
     let file = opencode_files::HandleFile {
@@ -349,6 +409,79 @@ fn a_handle_file_with_an_empty_credential_id_is_refused() {
     );
 }
 
+/// The Rust validator runs on the WRITE path (`write_handle_file_for_tenant`,
+/// `verify_handle_written`), so a rule it lacks lets `ck auth` ORIGINATE a row the
+/// TypeScript reader refuses -- and that reader refuses the whole document, denying
+/// every tenant in a shared file. These arms pin the two sides to one predicate.
+///
+/// Each case is also asserted in `packages/opencode/src/tests/contracts.test.ts`. The
+/// duplication is forced -- two languages, one contract -- so it is marked here rather
+/// than left to look like an independent local rule.
+///
+/// THE ARMS BELOW GO THROUGH `read_handle_file`, WHICH IS THE READ PATH. They pin the
+/// predicate but NOT the claim in the paragraph above: deleting `validate_handle_file`
+/// from the writer leaves every one of them green, because a raw fixture never reaches
+/// the writer at all. `an_invalid_handle_file_is_refused_at_the_write_path` is the arm
+/// that holds the writer, and it is separate for exactly that reason -- a comment
+/// asserting coverage its arms do not have is the defect this file keeps finding
+/// elsewhere.
+#[test]
+fn a_handle_file_with_a_cross_provider_credential_id_is_refused() {
+    let err = read_raw_handle_fixture(
+        "cross-provider-credential-id",
+        r#"{"version":1,"providers":[{"provider":"deepseek","shape":"api","serve":"opencode-claustrum","accounts":[{"label":"main","handle":"ckh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","credential_id":"oauth:anthropic"}]}]}"#,
+    )
+    .expect_err("a credential id scoped to another provider refuses");
+
+    assert!(
+        err.to_string().contains("invalid credential id"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn a_handle_file_with_an_empty_credential_id_segment_is_refused() {
+    for (name, credential_id) in [
+        ("empty-kind-segment", ":deepseek:main"),
+        ("empty-label-segment", "apikey:deepseek:"),
+        ("empty-middle-segment", "apikey::deepseek"),
+    ] {
+        let err = read_raw_handle_fixture(
+            name,
+            &format!(
+                r#"{{"version":1,"providers":[{{"provider":"deepseek","shape":"api","serve":"opencode-claustrum","accounts":[{{"label":"main","handle":"ckh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","credential_id":"{credential_id}"}}]}}]}}"#
+            ),
+        )
+        .unwrap_err();
+
+        assert!(
+            err.to_string().contains("invalid credential id"),
+            "{name}: unexpected error: {err}"
+        );
+    }
+}
+
+#[test]
+fn a_handle_file_with_live_credential_id_shapes_is_accepted() {
+    // Positive control against the real deployment: a tightening that refuses a live id
+    // is worse than the gap it closes, and every one of these is in the vault today.
+    for (provider, credential_id) in [
+        ("deepseek", "apikey:deepseek:main"),
+        ("anthropic", "oauth:anthropic"),
+        ("anthropic", "oauth:anthropic:work-alt"),
+        ("openai", "chatgpt:openai"),
+        ("google", "antigravity:google"),
+    ] {
+        read_raw_handle_fixture(
+            &format!("live-shape-{credential_id}"),
+            &format!(
+                r#"{{"version":1,"providers":[{{"provider":"{provider}","shape":"api","serve":"opencode-claustrum","accounts":[{{"label":"main","handle":"ckh_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","credential_id":"{credential_id}"}}]}}]}}"#
+            ),
+        )
+        .unwrap_or_else(|err| panic!("{credential_id} must parse: {err}"));
+    }
+}
+
 #[test]
 fn a_handle_file_with_a_malformed_superseded_capability_is_refused() {
     let err = read_raw_handle_fixture(

docs/opencode-custody-design.md

@@ -34,6 +34,48 @@ provider with in-request failover for providers the generic plugin serves.
 | Multi-account | One vault record per key/account; ordered priority list per provider |
 | Dedicated-plugin providers | Served by THEIR plugin consuming this client + handle file + tombstone convention; never by the generic closure |
 
+### Handle-manifest credential-id scope
+
+The handle manifest is multi-tenant: each tenant owns only its `provider` + `serve` blocks.
+Within a block for provider `P`, `credential_id.split(':')[1] === P` is the whole id-level
+scope check. Segment 1 is the credential kind and is an OPEN SET: `antigravity`, `apikey`,
+`chatgpt`, and `oauth` exist today, and new kinds are expected. Do not allowlist or infer the
+kind. Segment 3+ is a label convention only and is never consulted for provider scoping;
+provider scoping and label derivation are different properties, and an unlabelled
+`oauth:anthropic` is valid for `anthropic` just as `oauth:anthropic:any-label` is.
+
+| expectation | provider | account label | `credential_id` | reason |
+|---|---|---|---|---|
+| MUST RESOLVE | `openai` | `main` | `chatgpt:openai` | A kind-prefix rule would reject this live OpenAI shape. |
+| MUST RESOLVE | `google` | `main` | `antigravity:google` | A kind-prefix rule would reject this live Google shape. |
+| MUST RESOLVE | `anthropic` | `work-alt` | `oauth:anthropic:something-else` | The label must not be derived or consulted. |
+| MUST REJECT | `anthropic` | `main` | `chatgpt:openai` | A real cross-tenant id must not parse in another provider's block. |
+
+Provider-segment validation is a SHAPE check, not an existence check: only the runtime fence,
+which compares `credential_id` with what `credential.get` returns for the bound handle, proves
+that a binding names a real record. This rule is only a cheap pre-filter for cross-tenant
+smuggling. Tenant fixtures must therefore use the REAL vault credential id and say why; a tidier
+plausible id can pass every row above and still fail the runtime fence. Credential ids are
+operator-chosen and cannot be derived from the provider, account label, or record kind:
+`chatgpt:openai` is live even though its kind segment is `chatgpt` while the record kind is
+`oauth`; those are unrelated.
+
+**"The runtime fence catches it" is only a valid justification for a consumer that CAN read vault
+ground truth, and today that is a property of the transport a tenant happened to choose.** A tenant
+with its own transport reads `credential_id` off the `credential.get` reply and can refuse on
+mismatch. A tenant vendoring `@cortexkit/claustrum-client` CANNOT: `ServedCredential` is
+`{material, recordVersion, expiresAtMs}` and has never carried `credential_id` or `account_id` at
+any ref, so the client discards five of the eight non-secret fields the wire sends before a
+consumer sees them. This repo's own custody plugin is in that position — `packages/opencode/src/serve.ts`
+logs the manifest's `credential_id` beside the vault's `record_version`, two values from different
+sources that read as corroboration.
+
+That matters because relaxing a parse-time constraint is licensed by the runtime fence existing.
+That license was extended to three tenants while only two could exercise it. Until the client
+carries the served metadata, treat the fence as a per-tenant capability rather than a contract-level
+guarantee, and do not justify a parse-time relaxation by it without checking that the tenant in
+question can actually perform the comparison.
+
 ### Seam boundary
 
 This is a config-hook/fetch-seam integration, **not provider-universal custody**. The generic

packages/client/src/handles.ts

@@ -76,7 +76,45 @@ export function parseHandleFile(value: unknown): OpenCodeHandleFileV1 {
       if (labels.has(account.label)) invalid(`provider ${index} duplicates account label ${account.label}`)
       labels.add(account.label)
       if (!handleIsValid(account.handle)) invalid(`provider ${index} account ${account.label} has invalid handle`)
-      if (!account.credential_id) invalid(`provider ${index} account ${account.label} has invalid credential id`)
+      // Segment 2 of the credential id must BE the provider block it sits in. Without
+      // this the check was non-empty-string only, so an `oauth:openai` binding parsed
+      // cleanly inside an `anthropic` block -- a cross-provider smuggle that every
+      // tenant reading this manifest would have honoured. Two peer tenants found the
+      // same hole in their own parsers independently.
+      //
+      // SCOPED TO SEGMENT 2 ONLY, deliberately. Segment 1 (the kind) is an OPEN SET --
+      // `oauth:`, `chatgpt:`, `antigravity:`, `apikey:` are all live in this vault today
+      // -- so a kind allowlist would refuse real ids. Segment 3+ (the label) is
+      // operator-chosen and may be absent: main is the 2-segment `oauth:anthropic`,
+      // fallbacks are 3-segment. Constraining either would reject the deployment this
+      // contract describes.
+      //
+      // NO SEGMENT MAY BE EMPTY. Segment 2 alone is what fences the provider, but a
+      // position-1 check ignores the rest of the string, and that left two ids passing
+      // that name credentials which cannot exist: `:anthropic:x` (empty kind) and
+      // `oauth:anthropic:` (empty label) both satisfy "segment 2 is the provider"
+      // literally. Neither is a smuggle; both defer a GUARANTEED resolve-time failure
+      // past the door, and under custody a resolve-time failure on a tombstoned account
+      // is a dark route rather than a refused row.
+      //
+      // It also removes an asymmetry nobody designed and everyone would read as a bug:
+      // `oauth::anthropic` rejected while `:anthropic:x` passed, purely because the
+      // check indexed position 1 and ignored positions 0 and 2. Agreed with the peer
+      // tenant and mirrored on their side, so this is chosen rather than defaulted --
+      // the previous behaviour was two independent defaults that happened to differ.
+      //
+      // The emptiness guard is ALSO explicit rather than a consequence: `''.split(':')`
+      // yields `['']`, whose `[1]` is `undefined` and cannot equal a provider string,
+      // so an empty id would reject anyway -- but that is a coincidence doing
+      // load-bearing work, and the check this replaced (`!account.credential_id`) was
+      // the emptiness guard. NO TEST DISTINGUISHES THAT ONE (empty rejects with or
+      // without it, verified by removal), so it is kept for a future reader who loosens
+      // the comparison, not for an arm it could never redden. The non-empty-SEGMENT
+      // rule below is different: it reddens, and is pinned.
+      const segments = account.credential_id.split(':')
+      if (!account.credential_id || segments[1] !== item.provider || segments.some((segment) => segment.length === 0)) {
+        invalid(`provider ${index} account ${account.label} has invalid credential id`)
+      }
       if (account.superseded?.some((handle) => !handleIsValid(handle))) {
         invalid(`provider ${index} account ${account.label} has invalid superseded handle`)
       }

packages/opencode/README.md

@@ -38,6 +38,11 @@ The plugin never reads this table: only the CLI that creates tombstones does.
 
 The handle file comes from `CLAUSTRUM_OPENCODE_HANDLES`, or from `${XDG_CONFIG_HOME:-$HOME/.config}/cortexkit/opencode-handles.json`. It must be a regular file owned by the current user with mode `0600`; symlinks are refused. Provider ids and account labels must match `^[a-z0-9][a-z0-9._-]{0,63}$` and cannot be `__proto__`, `constructor`, or `prototype`. OpenCode auth is read from `OPENCODE_AUTH_CONTENT` when it is set, otherwise from `${XDG_DATA_HOME:-$HOME/.local/share}/opencode/auth.json`.
 
+The canonical handle-manifest credential-id scope contract and its conformance table live in
+`docs/opencode-custody-design.md` under “Handle-manifest credential-id scope”. In short, the
+second colon-separated segment must equal the block provider; the kind is open and labels are not
+used for validation.
+
 If the selected auth source cannot be parsed or validated, the plugin scans it in bounded chunks for self-describing tombstones and refuses the named providers. No scan hit leaves a never-migrated oversized auth source alone. A raw scan does not recognize JSON-escaped sentinel bytes; that hand-edit/foreign-writer limitation shares the same no-hit branch, so changing either behavior requires deciding both.
 
 OpenCode's provider API and UI serialize `Provider.Info.key`, so a tombstone can look like a configured credential. It is non-secret and does not grant access; custody still refuses when ownership cannot be proven.