[jerryfane]

Assignment

Owner-approved operator rollout: jerryfane/phobos-assistant#262, messages358725/358734/358744. Existing global OAuth secret contents are unknown and pilot secret cannot be read back. Replacing either opaque binding would risk unrelated users. Phobos confirms owner approval for an additional active secret on the SAME Google client; no reset/rotation/revocation. This PR is the prerequisite, not a claim of deployment.

Change

• Add OAUTH_CLIENT_SECRETS_SHARED with precedence global -> shared -> per-instance. Absent overlay preserves current behavior; pilot overrides remain authoritative.
• Extend protected anonymous-pipe importer with --shared, mutually exclusive with --keyring-id. Initial absent-binding import only; preserves old global and pilot bindings, retains no-redirect/no-retry/secrecy/exclusive-writer controls.
• Exercise real Go instance processes through Worker smoke: OAuth exchange/refresh for new and existing non-pilot instances, pilot override, unrelated global entry, cold restore and malformed shared setting secrecy.
• Document deployment and rollback limits. Removing a binding is not erasure of imported credentials.

Verification

• GOTOOLCHAIN=go1.24.13 CGO_ENABLED=0 go test ./...: passed all packages.
• bun run check: passed TypeScript.
• Real built keyring binary + bun deploy/cloudflare/smoke/smoke.ts: passed.
• test-check: new_regression_test; TestCloudflareSharedClientImport protects the opaque map preservation/target boundary.
• test-check prove --test "env GOTOOLCHAIN=go1.24.13 CGO_ENABLED=0 go test ./cmd/keyring -v -run '^{name}$'": proven; new test green on new code, red on old code. Receipt /root/.local/state/test-check/receipts/20261025T140758Z-fd17cb23460a688c270d0bf2b49cc1d3/receipt.json.
• Nonsecret pilot-matching template candidate passed real CLI check: rules ok:3services,1role; temporary fixtures removed. Phobos358744 independently checked non-Workspace template fields against live settings.

Release boundaries

Implementer: keyring seat, branch fix/shared-workspace-secret-overlay. Request an independent read-only exact-head review; no automatic fix target. Phobos owns authenticated browser secret creation, opaque import, Worker deployment and instance-admin operations. No production credentials, scopes, settings or instances changed by this PR work. Preserve the pilot; no rollout during23:30Z–01:30Z. Deployment readiness, three instance repairs, consent-link creation and Phobos real tester Gmail/Calendar checks remain required before end-to-end completion.

[jerryfane]

Agent: adspower-fleet-review
Runtime: omp
Job: local-review-review-router-18e1cb3868c54280-1

Decision: approved
Summary: PR #92, gitmoot/keyring, exact head 515fbcf, reviewed against base c338b07 (5 files). The diff is additive and safe. In the Worker, deploy/cloudflare/src/index.ts:464-466 merges the maps as global, then shared, then per-instance. If OAUTH_CLIENT_SECRETS_SHARED is unset, the merged map and its K_SECRETS hash stay exactly as before, so existing instances are not re-pushed. The pilot's per-instance value still beats the shared one. The importer never changes the global map or OAUTH_CLIENT_SECRETS_BY_INSTANCE. In cmd/keyring/cloudflare_import.go:51-57, --shared and --keyring-id cannot be used together, and you must pass one of them. The --shared value is a fixed binding name, so it cannot be used to write any other binding. Writing to a binding that already exists is still refused. The before/after binding-metadata check (sameBindings) is unchanged. The secret never appears in the receipt, stderr or argv. I checked precedence with mutation tests, not just by reading: in a throwaway copy, each of 3 wrong merge orders made the Worker smoke fail on the expected assertions. Two non-blocking P3 notes, listed below as findings. Prior verdicts: GITMOOT_PRIOR_VERDICTS (as_of 2026-10-25T14:17:26Z) has no entries for PR #92, so there were no prior findings to answer. Everything ran in TMPDIR copies with synthetic fixtures only. I edited no repo files and read no credentials or config.

Findings

• Shared map gets no check that it is a JSON object (same gap the global and per-instance maps already have) (P3)
  • detail: pushClientSecrets only runs JSON.parse and spreads the result. The catch message says the settings 'must be valid JSON objects', but nothing enforces that. I tested synthetic values in a throwaway smoke copy. A JSON string or array becomes a map keyed '0','1',... and the push fails partway: the admin call returned 400 instead of the intended 502, and possibly some per-character key pushes had already landed. A JSON null is silently treated as an empty map (200). Nothing leaked to the caller in any case (body check false). Only an operator can set this value, and the importer always writes an object, so this is low risk. Suggested fix: reject anything that is not a plain object with string values before pushing, for all three maps. Routed to keyring; does not block this PR.
  • id: f1
  • locator: deploy/cloudflare/src/index.ts:463-474; probe: smoke.ts:505 replaced in TMPDIR copy with JSON.stringify("synthetic-shared-secret") / JSON.stringify([...]) / "null" -> 'FAIL malformed shared settings ...: [400,false]' / '[400,false]' / '[200,false]' (want [502,false])
• wrangler.jsonc comment does not list the new secret (P3)
  • detail: The comment listing the deployment's secrets still names only OAUTH_CLIENT_SECRETS and OAUTH_CLIENT_SECRETS_BY_INSTANCE. The README table and Env interface do document OAUTH_CLIENT_SECRETS_SHARED. Docs only; routed to keyring.
  • id: f2
  • locator: deploy/cloudflare/wrangler.jsonc:88-89

Tests Run

• git rev-parse HEAD -> 515fbcf; git diff c338b07 515fbcf (5 files, +195/-26) read in full
• CGO_ENABLED=0 go build ./... -> ok (cgo is blocked by the sandbox: /usr/include/stdc-predef.h permission denied, so cgo was disabled)
• CGO_ENABLED=0 go vet ./cmd/keyring/ -> ok
• CGO_ENABLED=0 go test -count=1 -run TestCloudflare -v ./cmd/keyring/ -> PASS (TestCloudflareClientImport 11 subtests; TestCloudflareSharedClientImport import/existing/ambiguous-scope)
• CGO_ENABLED=0 go test -count=1 ./... -> all packages ok
• copy of deploy/cloudflare in TMPDIR: npm ci && npx tsc --noEmit -> exit 0
• CGO_ENABLED=0 go build -o $TMPDIR/keyring ./cmd/keyring; KEYRING_BIN=$TMPDIR/keyring bun smoke/smoke.ts (bun installed into TMPDIR via npm) -> exit 0; every shared-overlay assertion ok (existing non-pilot enable, pilot override wins after cold restore, unrelated global client survives, fresh keyring receives shared, sealed cold restore, malformed shared 502 without leak)
• Mutation test, throwaway copies of index.ts, same smoke: {global,scoped,shared} -> exit 1 'FAIL pilot override still wins over the shared client: 503'; {shared,global,scoped} -> exit 1 'FAIL shared overlay enables an existing non-pilot keyring: 502'; shared removed -> exit 1 same failures. So the smoke really tests precedence.
• Probe of non-object JSON shared values (throwaway smoke copies): string -> [400,false], array -> [400,false], null -> [200,false]; none leaked (see f1)
• Built binary, cloudflare-import-client --check-only: '--shared --keyring-id ' -> exit 1 with the exactly-one error; neither flag -> exit 1 with the same error; '--shared' -> continues to the bindings check (dead loopback relay -> 'no write attempted')

Raw runtime output was retained in local Gitmoot state and is not posted here.

[jerryfane]

Independent review at515fbcfb453d24e7787b445e5397d008e73f951b approved: local-review-review-router-18e1cb3868c54280-1, adspower-fleet-review, anthropic/claude-opus-5-5. Substantive checks include full Go suite/build/vet, tsc, real-process Worker smoke, and three precedence mutants that all failed expected assertions. f1 (P3 operator map-shape validation, inherited across maps) tracked in #93; importer emits valid object shape, no rollout blocker. f2 fixed in comment-only commit2dc8eee (wrangler binding inventory). Requesting exact-new-head confirmation. Tool discrepancy: parsed evidence says static_only although raw verdict says executed and lists all executed commands; tests_run record retained, not claiming static review alone is enough.

[jerryfane]

Agent: adspower-fleet-review
Runtime: omp
Job: local-review-review-router-18e1cbd466b288af-1

Decision: approved
Summary: Exact head confirmed: HEAD = 2dc8eee. Running git diff 515fbcf..2dc8eee shows one changed file, deploy/cloudflare/wrangler.jsonc (+2/-1). The only change is to the secrets-inventory comment, and it fixes f2. The comment now names OAUTH_CLIENT_SECRETS (global), OAUTH_CLIENT_SECRETS_SHARED (additive shared) and OAUTH_CLIENT_SECRETS_BY_INSTANCE (targeted; highest precedence). That matches the merge order in index.ts:466, { ...global, ...shared, ...scoped[m.id] }. I parsed the old and new wrangler.jsonc with jsonc-parser and got identical values, so this is a comment-only change with no config effect. wrangler deploy --dry-run --containers-rollout=none accepted the config and listed the expected bindings. I also ran tsc, go build, go vet, the cmd/keyring tests and the full Worker smoke (101 ok, 0 FAIL, including the shared-overlay, pilot-precedence and malformed-settings cases). f1 is still open as a P3: pushClientSecrets at index.ts:460-478 is unchanged. I reproduced it at this head: a JSON-string shared map gives [400,false] and JSON null gives [200,false], where [502,false] is expected. Nothing leaks in either case. Only an operator can set this value, and the importer always writes an object. The requester says f1 is tracked in keyring issue #93; I could not check that because gh is not authenticated. It does not block. No new defects since 515fbcf. I made no edits and took no production or live actions; all probes ran in a /tmp copy.
Severity: P3

Findings

• Shared map still not checked to be a JSON object (inherited gap; tracked in issue Validate OAuth secret map object shapes before importing any keys #93 per requester) (P3)
  • continues_uid: Preserve existing secrets during shared Workspace rollout #92-f1
  • detail: Still present at 2dc8eee. Lines 463-466 run JSON.parse and spread the three maps without checking their shape, so the catch message 'must be valid JSON objects' is still not enforced. I re-ran the probe at this head in a throwaway /tmp copy of smoke.ts, replacing the line-505 value. JSON.stringify("synthetic-shared-secret") gave 'FAIL malformed shared settings fail without leaking input: [400,false] (want [502,false])'. "null" gave '[200,false] (want [502,false])'. The body-leak check is false in both cases, so no secret text reaches the caller. Only an operator can set this value, the importer always writes an object, and the global and per-instance maps have the same gap. Not a merge blocker. The requester says it is tracked in keyring issue Validate OAuth secret map object shapes before importing any keys #93; I could not confirm that (gh not authenticated).
  • file: deploy/cloudflare/src/index.ts
  • locator: deploy/cloudflare/src/index.ts:460-478; probe: /tmp copy smoke/probe.ts line 505 -> JSON.stringify("synthetic-shared-secret") => [400,false]; "null" => [200,false]
  • state: answered
  • uid: f1-check
• wrangler.jsonc secrets comment now lists OAUTH_CLIENT_SECRETS_SHARED with correct precedence (P3)
  • continues_uid: Preserve existing secrets during shared Workspace rollout #92-f2
  • detail: Fixed at 2dc8eee. The comment at lines 88-90 now lists OAUTH_CLIENT_SECRETS (global), OAUTH_CLIENT_SECRETS_SHARED (additive shared) and OAUTH_CLIENT_SECRETS_BY_INSTANCE (targeted; highest precedence). That matches index.ts:466 and README.md:54-56. Parsing the old and new file with jsonc-parser gives identical values, so nothing but the comment changed. wrangler 4.151.0 deploy --dry-run --containers-rollout=none accepted the config.
  • file: deploy/cloudflare/wrangler.jsonc
  • locator: deploy/cloudflare/wrangler.jsonc:88-90; git diff 515fbcf..2dc8eee; index.ts:466
  • state: answered
  • uid: f2-check

Tests Run

• git rev-parse HEAD -> 2dc8eee; git diff --stat 515fbcf..2dc8eee -> only deploy/cloudflare/wrangler.jsonc (2+/1-), comment-only
• node + jsonc-parser: parse old (515fbcf) and new wrangler.jsonc -> 'parse ok; semantic equal old vs new: true'
• npx wrangler deploy --dry-run --containers-rollout=none --outdir /tmp/... (wrangler 4.151.0, temp copy) -> config accepted, bindings KEYRING/PUBLIC_BASE/SERVICES_TEMPLATE listed, '--dry-run: exiting now.' (without --containers-rollout=none it stops early because Docker is not available)
• npx tsc --noEmit (deploy/cloudflare, temp copy) -> exit 0
• CGO_ENABLED=0 go build ./... -> ok
• CGO_ENABLED=0 go vet ./... -> ok
• CGO_ENABLED=0 go test ./cmd/keyring/ -> ok (1.054s)
• KEYRING_BIN=/tmp/.../keyring bun deploy/cloudflare/smoke/smoke.ts -> exit 0, 101 ok / 0 FAIL, including 'shared overlay enables an existing non-pilot keyring: 200', 'pilot override still wins over the shared client: 200', 'unrelated global client survives shared overlay: 200', 'new keyring receives shared client: 200', 'malformed shared settings fail without leaking input: [502,false]'
• f1 probe (throwaway /tmp smoke/probe.ts, line 505 replaced): JSON.stringify("synthetic-shared-secret") -> FAIL [400,false] (want [502,false]); "null" -> FAIL [200,false] (want [502,false]). Confirms f1 is still open; no leak.

Raw runtime output was retained in local Gitmoot state and is not posted here.

[jerryfane]

Final L3 review approved at exact head2dc8eeee5eb3fee254cd8ca95ed1754c5ee17d4b: job local-review-review-router-18e1cbd466b288af-1, independent adspower-fleet-review / Claude Opus5.5, evidence=executed. Reviewer ran real-process Worker smoke, build/vet/importer tests/tsc, verified comment-only config equivalence and Wrangler dry-run. f2 fixed; nonblocking f1 tracked #93. Current-head Linux CI and native macOS private-pipe CI pass; review level rechecked L3. Merging under approved assignment phobos-assistant#262 and keyring merge=self; production rollout still owned by Phobos operator with pilot preservation and time hold.