feat(lab): CL-10 public evidence contract - #1510
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds privacy-safe public evidence projection, deterministic Ed25519-signed bundles, isolated community import and revocation, local CLI/API surfaces, purge integration, and read-only Compatibility Matrix context. It also adds an injectable startup-health dependency and configurable CI test timeouts. ChangesPublic evidence lifecycle
Management startup-health seam
CI test timeout controls
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟠 High · up to The current PR adds public evidence signing, import, privacy, revocation, and cache behavior, but the current head still permits unauthorized or incomplete evidence, privacy leakage, non-canonical signed bundles, revocation inconsistencies, quota bypass, and side effects on rejected requests. These are high-impact trust, security, and availability defects, so the PR is not safe to merge until the major issues are fixed and the contract/runtime scope is reconciled. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
documentation
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Contract review on exact head e07ea0949ad528c9b4de3d1c6469cf7852d0339d against current dev@cbbfdd877.
The overall privacy/trust direction is conservative and valuable: public export is an allowlist-only projection, private route dimensions make evidence non-exportable instead of being silently generalized, local identifiers and arbitrary diagnostics are excluded, signatures prove integrity/continuity rather than truth, explicit consent is required, community evidence remains non-authoritative, and remote publishing is blocked on a later fixed-origin service contract. I found no runtime behavior change because this PR changes documentation only, and exact-head docs/hygiene CI is green.
I am not approving the contract yet because two normative boundaries need to be made exact before implementation starts:
-
Canonicalization and signature domain are underspecified. The contract names deterministic canonical bytes and Ed25519 signatures, but it does not freeze a concrete canonicalization algorithm/encoding or state whether
bundleDigestandsignatureare excluded from the signed payload. It also does not freeze domain-separation bytes and verification order. Different serializers could produce incompatible or ambiguously self-referential bundles. Define one exact byte contract (for example, a named canonical JSON profile with UTF-8, integer/string rules, object-key ordering, and duplicate-key rejection), the exact preimage forbundleDigest, the exact signature preimage/domain tag, and the validation order. -
Revocation trust needs an anti-substitution rule. “The same publisher key” is necessary but not sufficient if an importer accepts a valid signed revocation whose publisher identity or target IDs are not bound to the original imported bundle's verified signer and schema domain. Require the revocation preimage to include a dedicated domain tag, schema/version, publisher key identity, target bundle/record IDs, and issued day; importers must apply it only when the verified key exactly matches the target bundle's verified publisher key. Also define deterministic conflict/replay handling for repeated or contradictory revocations.
Please update both the normative CL-10 contract and the shorter design summary so they do not diverge. After those byte-level and revocation bindings are explicit, this remains a strong contract candidate. This comment does not authorize runtime export/publish/import implementation or remote service selection.
✅ READY
UI screenshot waived by the Hygiene✅ Deterministic PR hygiene checks passed. |
Wibias
added
the
gui-screenshot-waived
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/pages/compatibility-matrix-api.ts`:
- Around line 315-323: Update fetchCommunityEvidenceContext and its callers so
community evidence is not fetched globally for each verdict detail: either pass
verdict.subjectId to the community endpoint and parse the subject-scoped
response, or fetch the global summary once at the page-level component and reuse
it. Ensure DetailPane displays totals corresponding to the selected verdict
rather than all records.
In `@gui/tests/compatibility-community-evidence.test.ts`:
- Around line 52-58: Replace the source-text assertions in the compatibility
test with a behavioral render assertion: render DetailPane twice using identical
verdict data, once with community set to null and once with populated community
context, then verify the rendered verdict cell output is byte-identical and the
community section includes the notLocalVerdict notice. Retain the existing
test-id and notice call-site guards if desired, but remove the broad
combined-score and community-verdict regex checks.
- Around line 41-50: Update compatibility-community-evidence.test.ts to derive
the locale iteration from the translation catalog rather than the hardcoded
LOCALES list. Reuse an exported catalog or locale list if available; otherwise
export a LAB_LOCALES array from lab-translations.ts and iterate it so every
catalog locale receives community-string coverage.
- Around line 27-39: Add negative assertions to the test using
parseCommunityEvidenceContext for the missing validation paths: unknown
top-level and evidence-item keys to exercise hasOnlyKeys, a non-hex bundleId, a
non-integer activeRecordCount, and an evidence array exceeding 4096 entries.
Cast extra-key fixtures to unknown as needed, and assert each malformed context
returns null.
In `@src/cli/lab.ts`:
- Around line 311-318: Update the "verify" branch of the lab CLI dispatcher to
inspect the result from verifyPublicEvidenceFile before returning, and raise a
nonzero-exit CLI error for every status except cryptographically_valid. Preserve
printing the verification summary, including JSON output, before failing; add a
test in the public lab surface suite that mutates an exported bundle byte and
asserts a nonzero exit code.
In `@src/lab/ledger/purge.ts`:
- Line 197: Update the purge flow containing purgeLocalPublicEvidenceCopies so
export validation or deletion failures are aggregated without aborting
independent artifact, ledger, and sqlite purge actions. Ensure the final result
still reports the export failure, and remove or replace the unused
scratch_export_delete_failed path unless the intended fail-closed behavior is
explicitly preserved and tested.
In `@src/lab/paths.ts`:
- Line 88: Remove the labPublicExportsDir alias and use labExportDir directly in
the public evidence purge and identity-discovery flows, including
purgeAllExports and localExportIdentities. Keep the shared <labRoot>/export
scope explicit at each call site and preserve existing behavior.
In `@src/lab/public/community.ts`:
- Around line 269-299: The community cache is repeatedly scanned and
cryptographically verified for each bundle and operation. In
src/lab/public/community.ts lines 269-299, update listCommunityEvidence and
verifiedRevocationsForBundle to obtain the directory listing and verified
revocation set once per call, then reuse them across bundles. In
src/lab/public/community.ts lines 226-248, update the bundle-target path to read
and verify only the deterministic bundle-<publisherKeyId>-<target.id>.json file,
and index record-target IDs once instead of requiring a unique full-cache scan.
- Around line 74-90: Update boundedInput so the object branch retains its
pre-serialization scanStructure(raw) check, then canonicalizes jcsStringify(raw)
and passes the resulting bytes through parseStrictPublicJson before returning.
Ensure both byte/string and object inputs produce the same strictly parsed
representation while preserving the existing size check and early structural
validation.
In `@src/lab/public/operator.ts`:
- Around line 138-179: Update the selection flow around
latestObservationCompletedAt and projectPublicEvidence so createdDayUtc is
derived only from observations that produce exported records, computing it after
projection from included inputs. Remove the pre-exclusion timestamp
accumulation, and distinguish no observation events selected from all
observations being excluded so the latter preserves and surfaces per-event
exclusion reasons rather than throwing public_selection_empty.
- Around line 112-126: Update canonicalVerdictForObservation to paginate
queryLabVerdicts results using nextCursor, continuing until a matching
suiteVersion and contributing event ID is found or hasMore is false. Preserve
the existing query filters and return null only after all available pages have
been searched.
In `@src/lab/public/project.ts`:
- Around line 81-105: Update the project-to-signing/storage flow around the
PublicEvidenceRecordV1 construction and validateCommunityEvidenceRecord so
validateCommunityEvidenceAuthorities() and a fail-closed privacy scan run over
the complete evidence bundle before any local signing or storage; reject
unauthorised records and secret-shaped identifiers, and add regression coverage
for both rejection paths.
In `@src/lab/public/purge.ts`:
- Around line 103-138: Update src/lab/public/purge.ts lines 103-138 so
purgeAllExports runs before community-bundle processing, unreadable files are
tolerated by localExportIdentities, and deletion relies only on lstatSync
path-safety checks rather than readCommunityEvidenceBundleForPublisher
verification. Update src/lab/public/purge.ts line 104 so
readExistingPublisherKeyId returns null for unsafe or permission-denied key
files, allowing provenance classification to degrade without aborting the purge.
Apply the same fix in `@src/lab/public/purge.ts` at line 104.
In `@src/lab/public/revocation.ts`:
- Around line 112-113: Centralize publisher private-key access: in
src/lab/public/revocation.ts lines 112-113, update the signing flow around
signBytes to use signature.ts’s hardened readRestrictedPrivateKey or an
already-validated PEM from PublicPublisherHandle instead of rereading
handle.privateKeyPath; in src/lab/paths.ts lines 94-96, derive the filename from
the shared PUBLISHER_KEY_FILE constant rather than duplicating the
publisher-ed25519.pem literal.
In `@src/lab/public/signature.ts`:
- Around line 47-61: Eliminate path re-resolution by validating and consuming
the same open descriptor at all sites: in src/lab/public/signature.ts:47-61,
update readRestrictedPrivateKey to use openSync/fstatSync, read from the
descriptor, and close it in finally; in src/lab/public/operator.ts:217-230,
update readBoundedPublicFile similarly while retaining both size checks; in
src/lab/public/storage.ts:85-93, update readPublicEvidenceBundle to validate and
read through one descriptor; and in src/lab/public/storage.ts:48-55, replace
existsSync/lstatSync/readFileSync with one open attempt, treating ENOENT as
absent before the exclusive-create branch and comparing bytes through the
already validated descriptor.
In `@src/lab/public/storage.ts`:
- Around line 94-96: Validate the result of parseStrictPublicJson as a non-null
object before treating it as PublicEvidenceBundleV1, and reject invalid values
using the module’s existing validation error type. Only perform the bundleId
comparison and assertLocalArtifactExportAuthority call after this shape check,
preserving normal processing for valid object exports.
In `@src/lab/public/strict-json.ts`:
- Around line 123-146: Bound recursive JSON parsing in parseValue, parseObject,
and parseArray with a deterministic maximum nesting depth, rejecting inputs that
exceed it via the existing PublicEvidenceValidationError path rather than
allowing native stack overflow. Add a focused regression test beside the
duplicate-key test in tests/lab-public-wire-contract.test.ts that submits a
deeply nested body and asserts PublicEvidenceValidationError.
In `@src/server/management/lab-routes.ts`:
- Around line 265-271: Update publicErrorResponse so only
PublicEvidenceValidationError returns its validation code, message, and HTTP 400
response. Map all other errors to a generic "server_error" response with a safe
internal-failure message and HTTP 500, without exposing err.message; preserve
the existing validation response contract used by the public routes.
In `@tests/lab-community-evidence.test.ts`:
- Around line 165-185: Add a focused regression test alongside the existing
purge tests that imports a bundle signed by the consumer directory without
calling writePublicEvidenceBundle, then runs purgeSensitiveEvidence with the
export action and verifies listCommunityEvidence returns no bundles. Ensure the
setup leaves only the publisher-key condition true, so the test specifically
covers same-publisher classification.
- Around line 139-163: Rename the existing test to cover digest-invalid same-ID
data, then add a focused test for the `community_conflict` branch in
`importCommunityEvidenceRevocation`: after storing a valid revocation, overwrite
its stored file while preserving the filename identity but changing its
serialized bytes, then re-import the genuine revocation and assert the
different-bytes error. Add `writeFileSync` to the existing `node:fs` imports.
In `@tests/lab-public-wire-contract.test.ts`:
- Around line 13-17: Update the allowlisting around FIXED_PRIVATE_KEY in the
test so Betterleaks recognizes the deterministic test vector without modifying
the PEM contents. Place the bare betterleaks:allow marker on a line covered by
the finding, or add a rule-specific allowlist entry targeting finding.Line.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5c537599-f90b-4932-8627-39e9e15c46a6
📒 Files selected for processing (38)
devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.mddocs/superpowers/plans/2026-08-12-cl10-public-evidence-implementation.mddocs/superpowers/specs/2026-08-12-cl10-public-evidence-design.mdgui/src/i18n/lab-translations.tsgui/src/pages/CompatibilityMatrix.tsxgui/src/pages/compatibility-matrix-api.tsgui/tests/compatibility-community-evidence.test.tssrc/cli/lab.tssrc/lab/index.tssrc/lab/ledger/purge.tssrc/lab/paths.tssrc/lab/public/bundle.tssrc/lab/public/community-authority.tssrc/lab/public/community.tssrc/lab/public/ids.tssrc/lab/public/index.tssrc/lab/public/operator.tssrc/lab/public/project.tssrc/lab/public/purge.tssrc/lab/public/registry.tssrc/lab/public/revocation.tssrc/lab/public/signature.tssrc/lab/public/storage.tssrc/lab/public/strict-json.tssrc/lab/public/types.tssrc/lab/public/validate.tssrc/server/management/config-routes.tssrc/server/management/context.tssrc/server/management/lab-routes.tstests/lab-community-evidence.test.tstests/lab-community-publisher-continuity.test.tstests/lab-public-api-json.test.tstests/lab-public-artifact-policy.test.tstests/lab-public-evidence.test.tstests/lab-public-surfaces.test.tstests/lab-public-wire-contract.test.tstests/settings-startup-health-seam.test.tstests/settings-stream-mode.test.ts
| test("Compatibility Matrix renders community evidence as separate context, never a combined score", async () => { | ||
| const source = await Bun.file(new URL("../src/pages/CompatibilityMatrix.tsx", import.meta.url)).text(); | ||
| expect(source).toContain('data-testid="lab-community-evidence"'); | ||
| expect(source).toContain('labSupplement(locale, "community.notLocalVerdict")'); | ||
| expect(source).not.toMatch(/combined.?score/i); | ||
| expect(source).not.toMatch(/community.*verdict\s*=|verdict\s*=.*community/i); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
The third test asserts source text, so it does not verify the isolation property it claims, and it can fail on unrelated edits.
Line 53 reads CompatibilityMatrix.tsx as a string, then Lines 56-57 assert the absence of two regular expressions. Both directions are unreliable:
- False negatives. Line 57 uses
/community.*verdict\s*=|verdict\s*=.*community/i. The.metacharacter does not match a newline, so the match is effectively line-scoped. A real violation such as assigning a community-derived value into verdict state across two lines passes. A rename tocommunityScorealso passes both regexes while doing exactly what the test name forbids. - False positives. Line 56 asserts
not.toMatch(/combined.?score/i)against the entire file including comments. A future comment that says "do not produce a combined score" would fail the test, and the failure message would point at nothing actionable.
The invariant worth protecting is behavioral: community evidence must never change the rendered local verdict. Assert that behaviorally instead. Render DetailPane twice with identical verdict data, once with community: null and once with a populated context, then assert the verdict cell output is byte-identical and that the community section carries the notLocalVerdict notice.
Keep Lines 54-55 if you want a cheap guard that the test id and the notice call site still exist. Replace Lines 56-57 with the behavioral assertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/tests/compatibility-community-evidence.test.ts` around lines 52 - 58,
Replace the source-text assertions in the compatibility test with a behavioral
render assertion: render DetailPane twice using identical verdict data, once
with community set to null and once with populated community context, then
verify the rendered verdict cell output is byte-identical and the community
section includes the notLocalVerdict notice. Retain the existing test-id and
notice call-site guards if desired, but remove the broad combined-score and
community-verdict regex checks.
| const FIXED_PRIVATE_KEY = `-----BEGIN PRIVATE KEY----- | ||
| MC4CAQAwBQYDK2VwBCIEIAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4f | ||
| -----END PRIVATE KEY----- | ||
| `; | ||
| const FIXED_PUBLIC_KEY = "MCowBQYDK2VwAyEAA6EHv/POEL4dcN0Y50vAmWfk1jCbpQ1fHdyGZBJVMbg="; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Betterleaks inline allowlist comment syntax suppress finding
💡 Result:
To suppress a finding in Betterleaks using an inline comment, you can use the following syntax [1][2]: # betterleaks:allow Placing this comment on the same line as a potential secret or immediately preceding it instructs the scanner to ignore that specific match [1][2]. Additionally, Betterleaks supports backwards compatibility with Gitleaks by also recognizing [2]: # gitleaks:allow If you wish to ignore these types of inline comments entirely, you can use the following flag during execution [3]: --ignore-gitleaks-allow
Citations:
- 1: https://medium.com/devsecops-ai/i-switched-from-gitleaks-to-betterleaks-heres-what-changed-de286168bf9c
- 2: https://github.com/betterleaks/betterleaks/blob/1ee7898e728f8612e39b97833c6caa9b4f5a8e41/detect/detect.go
- 3: https://github.com/oxsecurity/megalinter/blob/main/docs/descriptors/repository_betterleaks.md
🏁 Script executed:
#!/bin/sh
set -eu
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(tests/lab-public-wire-contract\.test\.ts|\.betterleaks|betterleaks|gitleaks|secret|security|package\.json|.*ya?ml)$' || true
printf '%s\n' '--- test lines ---'
sed -n '1,105p' tests/lab-public-wire-contract.test.ts
printf '%s\n' '--- scanner references ---'
rg -n -i --glob '!node_modules' --glob '!dist' --glob '!build' 'betterleaks|gitleaks|secret.?scan|private-key|allow' .github . 2>/dev/null | head -200 || trueRepository: lidge-jun/opencodex
Length of output: 28587
🌐 Web query:
site:github.com/betterleaks/betterleaks "betterleaks:allow" inline comment syntax
💡 Result:
Yes, Betterleaks supports inline comment syntax to ignore findings [1]. You can use the betterleaks:allow comment tag within your code to suppress specific alerts [1]. Additionally, for backward compatibility, Betterleaks also supports the gitleaks:allow comment tag [1]. The implementation logic confirms that these signatures are checked during the detection process to determine if a finding should be ignored [1]. While Betterleaks provides these inline options, you can also manage false positives globally or per-rule using the expressive Expr-based filter system defined in your configuration file [2][3].
Citations:
- 1: https://github.com/betterleaks/betterleaks/blob/1ee7898e728f8612e39b97833c6caa9b4f5a8e41/detect/detect.go
- 2: https://www.github.com/betterleaks/betterleaks
- 3: https://github.com/betterleaks/betterleaks
🏁 Script executed:
#!/bin/sh
set -eu
url='https://raw.githubusercontent.com/betterleaks/betterleaks/1ee7898e728f8612e39b97833c6caa9b4f5a8e41/detect/detect.go'
curl -fsSL "$url" | sed -n '1,260p' | rg -n -C 8 'betterleaks:allow|gitleaks:allow|Allow|allow'
python3 - <<'PY'
from pathlib import Path
p = Path("tests/lab-public-wire-contract.test.ts")
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if "FIXED_PRIVATE_KEY" in line or "BEGIN PRIVATE KEY" in line or "MC4CAQ" in line:
print(f"{i}: {line}")
PY
printf '%s\n' '--- detector references in fetched source ---'
curl -fsSL "$url" | rg -n 'allow|gitleaks|betterleaks|line' | head -100Repository: lidge-jun/opencodex
Length of output: 8318
🏁 Script executed:
#!/bin/sh
set -eu
url='https://raw.githubusercontent.com/betterleaks/betterleaks/1ee7898e728f8612e39b97833c6caa9b4f5a8e41/detect/detect.go'
curl -fsSL "$url" | sed -n '500,620p'Repository: lidge-jun/opencodex
Length of output: 3982
🏁 Script executed:
#!/bin/sh
set -eu
api='https://api.github.com/repos/betterleaks/betterleaks/git/trees/1ee7898e728f8612e39b97833c6caa9b4f5a8e41?recursive=1'
curl -fsSL "$api" |
jq -r '.tree[] | select(.type == "blob" and (.path | test("(_test\\.go$|test|allow|gitleaks)"; "i"))) | .path' |
head -100
printf '%s\n' '--- source references to allow-signature tests ---'
curl -fsSL "$api" |
jq -r '.tree[] | select(.type == "blob" and (.path | endswith("_test.go"))) | .path' |
while IFS= read -r path; do
content=$(curl -fsSL "https://raw.githubusercontent.com/betterleaks/betterleaks/1ee7898e728f8612e39b97833c6caa9b4f5a8e41/$path") || continue
printf '%s\n' "$content" | rg -q -i 'betterleaks:allow|gitleaks:allow' && {
printf '%s\n' "--- $path ---"
printf '%s\n' "$content" | rg -n -C 8 -i 'betterleaks:allow|gitleaks:allow'
}
doneRepository: lidge-jun/opencodex
Length of output: 5197
Use a valid Betterleaks allowlist
tests/lab-public-wire-contract.test.ts:13-16 contains a deterministic PKCS#8 Ed25519 test vector with seed 00 01 02 ... 1f, not a production secret. Betterleaks accepts the bare betterleaks:allow token and checks finding.Line; a comment before FIXED_PRIVATE_KEY will not suppress a finding on the multiline PEM. Place the marker on a line included in the finding, or add a rule-specific allowlist entry without changing the PEM.
🧰 Tools
🪛 Betterleaks (1.7.3)
[high] 13-15: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-wire-contract.test.ts` around lines 13 - 17, Update the
allowlisting around FIXED_PRIVATE_KEY in the test so Betterleaks recognizes the
deterministic test vector without modifying the PEM contents. Place the bare
betterleaks:allow marker on a line covered by the finding, or add a
rule-specific allowlist entry targeting finding.Line.
Source: Linters/SAST tools
Wibias
changed the title
github-actions
Bot
added
enhancement
0366f16 to
8906134
Compare
8906134 to
1d75f83
Compare
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (2)
src/cli/lab.ts (1)
311-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThrow
LabStateErrorfor failed public verificationAt
src/cli/lab.ts:317-319, throwLabStateErrorinstead ofError.runCliActionmapsRuntimeApiErrorto exit code 1 without printingUSAGE(src/cli/runtime-api.ts:321-324). Add a regression test intests/lab-public-surfaces.test.tsfor a tampered bundle and assert a nonzero exit code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/lab.ts` around lines 311 - 321, Update the failed-status branch in the “verify” case of the lab CLI to throw LabStateError instead of Error, preserving the existing failure message and successful verification flow. Add a regression test in lab-public-surfaces.test.ts that verifies a tampered bundle exits with a nonzero status.Source: Path instructions
src/lab/public/community.ts (1)
73-85: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
boundedInputcanonicalizes an untrusted object before it applies any bound.Lines 74-78 pick the byte source. For the object branch,
jcsStringify(raw)runs first. TheMAX_IMPORT_BYTEScheck at line 79 andscanStructureat line 83 both run after that serialization.
src/lab/public/operator.tsexposesimportCommunityEvidenceValue(raw), so an already-decoded value reaches this branch. A deeply nested object then drivesjcsStringifyrecursion with no depth bound, which can exhaust the stack, and a wide object allocates its full canonical string before the 2 MiB check rejects it. The byte and string branches are unaffected because their length is known before parsing.Move
scanStructure(raw)ahead of the serialization for the object branch.MAX_DEPTH,MAX_OBJECT_KEYS, andMAX_ARRAY_ELEMENTSthen bound the input beforejcsStringifywalks it.🛡️ Proposed ordering fix
function boundedInput(raw: unknown): unknown { - const bytes = raw instanceof Uint8Array - ? Buffer.from(raw) - : typeof raw === "string" - ? Buffer.from(raw, "utf8") - : Buffer.from(jcsStringify(raw), "utf8"); + let bytes: Buffer; + if (raw instanceof Uint8Array) { + bytes = Buffer.from(raw); + } else if (typeof raw === "string") { + bytes = Buffer.from(raw, "utf8"); + } else { + // Bound depth and width before canonical serialization walks the value. + scanStructure(raw); + bytes = Buffer.from(jcsStringify(raw), "utf8"); + } if (bytes.byteLength > MAX_IMPORT_BYTES) { throw new PublicEvidenceValidationError("community_size", "community import exceeds 2 MiB"); } const parsed = parseStrictPublicJson(bytes, "community import"); scanStructure(parsed); return parsed; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lab/public/community.ts` around lines 73 - 85, Update boundedInput so non-byte, non-string objects are passed to scanStructure(raw) before jcsStringify(raw) creates the byte buffer, while preserving the existing byte-size check and parsed-value scan for serialized inputs. Ensure MAX_DEPTH, MAX_OBJECT_KEYS, and MAX_ARRAY_ELEMENTS validate the raw object before canonicalization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/tests/compatibility-community-evidence.test.ts`:
- Around line 87-141: Add focused tests alongside the existing community
evidence test for fetchLabPageData: make the community endpoint reject with a
non-abort error and assert the page still resolves with community set to null,
then make it reject with the request AbortSignal’s abort error and assert that
fetchLabPageData propagates the abort. Preserve the existing success-path
coverage and reuse the current fetch stubbing and cleanup pattern.
In `@src/lab/ledger/purge.ts`:
- Around line 206-222: Build and validate the purge tombstone after the deletion
block, using the successfully populated completed action set instead of the
requested purgeActions, so a failed purgeLocalPublicEvidenceCopies call never
records export as completed. Preserve the existing deferred error behavior and
add a focused regression test beside the existing export-only purge case in
tests/lab-community-evidence.test.ts, asserting the committed tombstone omits
export when export deletion fails.
In `@src/lab/public/community-authority.ts`:
- Around line 24-37: Update validateAssertionAuthority to perform a reverse
check after validating record assertions: for every entry in assertions with
required: true, verify that record.assertions contains the same assertion id and
required flag, and throw PublicEvidenceValidationError with the existing
public_authority category when missing. Preserve the current rejection of
unrecognized or mismatched record assertions.
- Around line 39-90: Cache the immutable results of loadCaseAuthority(),
loadFabricCaseAuthority(), and verifierManifestDigest() so bundle validation
does not repeat synchronous I/O and recomputation for every record. Update
validateTaskAuthority and validateScenarioAuthority, plus
validatePublicEvidenceAuthorities or its validation context, to reuse the cached
values while preserving all existing authority checks.
In `@src/lab/public/community.ts`:
- Around line 135-157: Update persistAt to track that the file was created by
the current call and, when writeAll, fsyncSync, or assertRegular fails after
creation, close the descriptor, unlink the newly created path with unlinkSync,
and rethrow the original error. Preserve the existing EEXIST comparison flow for
files created by another process, and add unlinkSync to the node:fs imports.
In `@src/lab/public/operator.ts`:
- Around line 229-232: Update the export flow around writePublicEvidenceBundle
so it returns and propagates the authoritative created flag from its
O_CREAT|O_EXCL result, including the EEXIST path. Remove the racy existsSync
probe and expectedPath construction from the surrounding function, and use the
writer’s returned path and created value when building stored.
- Around line 249-267: Update readBoundedPublicFile to reject symlink paths
before opening them: use lstatSync on the requested path and throw
PublicEvidenceValidationError with the existing unsafe-file classification when
it reports a symbolic link. Preserve the O_NOFOLLOW open behavior and the
stats.nlink !== 1 hardlink check, while removing the ineffective
stats.isSymbolicLink() check from the fstatSync result.
- Around line 36-40: Remove the deprecated createdDayUtc property from
ProjectPublicEvidenceInput so callers cannot provide a silently ignored value;
leave projectPublicEvidence’s latestExportableCompletedAt-based derivation
unchanged.
In `@src/lab/public/privacy.ts`:
- Around line 74-83: Update validatePublicEvidencePrivacy to cover artifact
content: for text-like artifact media types, decode contentBase64 and pass the
decoded text through assertPrivacySafeString; until that scan is implemented,
reject any non-empty artifacts array rather than allowing unscanned bytes. Add a
regression test placing a credential canary in artifact content and verify
validation rejects it.
In `@src/lab/public/project.ts`:
- Around line 53-63: The exported projectPublicEvidenceRecord must enforce
authority and privacy validation before returning an exportable record. Move
validatePublicEvidenceAuthorities and validatePublicEvidenceRecordPrivacy into
this function, preserving the existing shape and identifier validation, and add
a test confirming a direct projector call rejects a privacy-unsafe scenarioId.
- Around line 21-26: Extract the shared utcDay helper used by
src/lab/public/project.ts and src/lab/public/operator.ts, validating finite,
non-negative timestamps within the Date-supported range and throwing
PublicEvidenceValidationError for invalid values. Remove the duplicate operator
implementation and update both modules to import the shared helper from the
appropriate validation module, preserving the existing UTC date conversion.
In `@src/lab/public/purge.ts`:
- Around line 89-113: Update the community cleanup loop in
purgeLocalPublicEvidenceCopies and unlinkLocalCommunityFile so unsafe community
entries, including hardlinked files, are skipped rather than propagated as
errors; continue processing remaining entries and preserve the successful
deletedExports result. Remove the now-unused PublicEvidenceValidationError
dependency if no longer referenced.
In `@src/lab/public/registry.ts`:
- Around line 8-23: Update PUBLIC_ROUTE_REGISTRY_SOURCE_COMMIT in the manifest
metadata to a reviewed, reachable commit whose tree contains the authority state
and registry.ts. In the entries definition, remove "openai-chat" from the openai
provider’s adapterFamilies, retaining only "openai-responses" for gpt-5.6-sol.
In `@src/lab/public/strict-json.ts`:
- Around line 79-141: Update parseArray and parseObject to count elements and
keys while scanning, rejecting arrays over 512 elements and objects over 64 keys
via the existing PublicEvidenceValidationError path before JSON.parse or
semantic validation. Preserve duplicate-key and nesting checks, and add focused
regression tests in the existing lab public wire-contract test suite for
over-wide arrays and objects.
In `@src/lab/public/validate.ts`:
- Around line 27-29: Replace the generated count-based
PUBLIC_INCIDENT_CORPUS_IDS definition with a code-owned authority set containing
the supported incident identifiers, and make isPublicIncidentRef use that set.
Add a parity test that verifies the authority set matches the 21 documented
identifiers from IC-001 through IC-021, so corpus changes cannot silently drift.
In `@tests/settings-startup-health-seam.test.ts`:
- Around line 26-29: Update the getCachedStartupHealth test double in
tests/settings-startup-health-seam.test.ts (lines 26-29) to return a complete,
properly typed StartupHealth fixture without as never. Apply the same shared
complete typed fixture in tests/settings-stream-mode.test.ts (lines 35-37), and
assert against a valid distinguishing StartupHealth field there.
---
Duplicate comments:
In `@src/cli/lab.ts`:
- Around line 311-321: Update the failed-status branch in the “verify” case of
the lab CLI to throw LabStateError instead of Error, preserving the existing
failure message and successful verification flow. Add a regression test in
lab-public-surfaces.test.ts that verifies a tampered bundle exits with a nonzero
status.
In `@src/lab/public/community.ts`:
- Around line 73-85: Update boundedInput so non-byte, non-string objects are
passed to scanStructure(raw) before jcsStringify(raw) creates the byte buffer,
while preserving the existing byte-size check and parsed-value scan for
serialized inputs. Ensure MAX_DEPTH, MAX_OBJECT_KEYS, and MAX_ARRAY_ELEMENTS
validate the raw object before canonicalization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aab75cb5-408b-4e44-ad00-948add4b2ace
📒 Files selected for processing (39)
devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.mddocs/superpowers/plans/2026-08-12-cl10-public-evidence-implementation.mddocs/superpowers/specs/2026-08-12-cl10-public-evidence-design.mdgui/src/i18n/lab-translations.tsgui/src/pages/CompatibilityMatrix.tsxgui/src/pages/compatibility-matrix-api.tsgui/tests/compatibility-community-evidence.test.tssrc/cli/lab.tssrc/lab/index.tssrc/lab/ledger/purge.tssrc/lab/paths.tssrc/lab/public/bundle.tssrc/lab/public/community-authority.tssrc/lab/public/community.tssrc/lab/public/ids.tssrc/lab/public/index.tssrc/lab/public/operator.tssrc/lab/public/privacy.tssrc/lab/public/project.tssrc/lab/public/purge.tssrc/lab/public/registry.tssrc/lab/public/revocation.tssrc/lab/public/signature.tssrc/lab/public/storage.tssrc/lab/public/strict-json.tssrc/lab/public/types.tssrc/lab/public/validate.tssrc/server/management/config-routes.tssrc/server/management/context.tssrc/server/management/lab-routes.tstests/lab-community-evidence.test.tstests/lab-community-publisher-continuity.test.tstests/lab-public-api-json.test.tstests/lab-public-artifact-policy.test.tstests/lab-public-evidence.test.tstests/lab-public-surfaces.test.tstests/lab-public-wire-contract.test.tstests/settings-startup-health-seam.test.tstests/settings-stream-mode.test.ts
| function validateTaskAuthority(record: PublicEvidenceRecordV1): void { | ||
| const fabricAuthority = loadFabricCaseAuthority(); | ||
| const caseRecord = fabricAuthority.cases.find((candidate) => candidate.id === FABRIC_SCENARIO_ID); | ||
| if ( | ||
| !caseRecord | ||
| || record.suiteId !== FABRIC_SUITE_ID | ||
| || record.suiteVersion !== FABRIC_SUITE_VERSION | ||
| || record.scenarioId !== FABRIC_SCENARIO_ID | ||
| || record.scenarioVersion !== FABRIC_SCENARIO_VERSION | ||
| || record.subject.subjectKind !== "task" | ||
| || record.subject.taskClassId !== FABRIC_TASK_CLASS_ID | ||
| || record.subject.taskClassVersion !== FABRIC_TASK_CLASS_VERSION | ||
| || record.subject.taskFixtureDigest !== caseRecord.fixture.digest | ||
| || record.subject.verifierManifestDigest !== verifierManifestDigest() | ||
| || record.subject.fabricCompatibilityVersion !== FABRIC_COMPATIBILITY_VERSION | ||
| ) { | ||
| throw new PublicEvidenceValidationError("public_authority", "task scenario/verifier authority mismatch"); | ||
| } | ||
| validateAssertionAuthority(record, caseRecord.assertions); | ||
| validateRouteAuthority(record.subject.route); | ||
| } | ||
|
|
||
| function validateScenarioAuthority(record: PublicEvidenceRecordV1): void { | ||
| if (record.evidenceLayer === "task_effectiveness") { | ||
| validateTaskAuthority(record); | ||
| return; | ||
| } | ||
|
|
||
| const authority = loadCaseAuthority(); | ||
| const caseRecord = authority.cases.find((candidate) => candidate.id === record.scenarioId); | ||
| if ( | ||
| !caseRecord | ||
| || caseRecord.suite !== record.suiteId | ||
| || record.scenarioVersion !== String(authority.manifestDefaults.version) | ||
| || record.suiteVersion !== String(authority.manifestDefaults.suiteVersion) | ||
| ) { | ||
| throw new PublicEvidenceValidationError("public_authority", "scenario/suite authority mismatch"); | ||
| } | ||
| validateAssertionAuthority(record, caseRecord.assertions); | ||
|
|
||
| if (record.evidenceLayer === "live_route_compatibility") { | ||
| if (record.subject.subjectKind !== "route") { | ||
| throw new PublicEvidenceValidationError("public_authority", "live route subject mismatch"); | ||
| } | ||
| validateRouteAuthority(record.subject); | ||
| } | ||
| } | ||
|
|
||
| /** Repository-owned authority gate used by both local signing and community imports. */ | ||
| export function validatePublicEvidenceAuthorities(records: readonly PublicEvidenceRecordV1[]): void { | ||
| for (const record of records) validateScenarioAuthority(record); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the three authority loaders for caching.
ast-grep run --pattern 'export function loadCaseAuthority($$$) { $$$ }' --lang typescript src/lab
ast-grep run --pattern 'export function loadFabricCaseAuthority($$$) { $$$ }' --lang typescript src/lab
ast-grep run --pattern 'export function verifierManifestDigest($$$) { $$$ }' --lang typescript src/labRepository: lidge-jun/opencodex
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -i 'community-authority|community|manifest|authority|verifier' src/lab -t f | sort
printf '%s\n' '--- loader definitions and references ---'
rg -n --glob '*.ts' '\b(loadCaseAuthority|loadFabricCaseAuthority|verifierManifestDigest|validatePublicEvidenceAuthorities|listCommunityEvidence)\b' src/lab
printf '%s\n' '--- outlines for relevant files ---'
for f in $(fd -i 'community-authority|community|manifest|authority|verifier' src/lab -t f | sort); do
printf '\n### %s (%s lines)\n' "$f" "$(wc -l < "$f")"
ast-grep outline "$f" 2>/dev/null | head -120 || true
doneRepository: lidge-jun/opencodex
Length of output: 13409
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- loader definitions ---'
rg -n -U --glob '*.ts' \
'(^|\n)(export )?(async )?function (loadCaseAuthority|loadFabricCaseAuthority|verifierManifestDigest)\b|(^|\n)(export )?const (loadCaseAuthority|loadFabricCaseAuthority|verifierManifestDigest)\b' \
src
printf '%s\n' '--- target implementation ---'
cat -n src/lab/public/community-authority.ts | sed -n '1,130p'
printf '%s\n' '--- bundle and community limits/callers ---'
rg -n -C 8 --glob '*.ts' \
'MAX_PUBLIC_BUNDLE_RECORDS|MAX_CACHE_FILES|listCommunityEvidence|validatePublicEvidenceAuthorities' \
src/labRepository: lidge-jun/opencodex
Length of output: 21877
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- files ---'
git ls-files 'src/lab' | grep -E '(community|authority|manifest|verifier)' | sort
printf '%s\n' '--- symbols ---'
rg -n --glob '*.ts' 'loadCaseAuthority|loadFabricCaseAuthority|verifierManifestDigest|validatePublicEvidenceAuthorities|listCommunityEvidence' .Repository: lidge-jun/opencodex
Length of output: 11340
🌐 Web query:
lidge-jun/opencodex src/lab/public/community-authority.ts loadCaseAuthority loadFabricCaseAuthority verifierManifestDigest
💡 Result:
The project lidge-jun/opencodex is a local proxy designed to allow users to integrate various Large Language Models (LLMs) with OpenAI Codex, Claude Code, and related tooling [1][2][3]. Regarding the specific identifiers and paths mentioned: 1. src/lab/public/community-authority.ts: This path appears to be associated with an experimental or internal component (indicated by lab/public) within the opencodex codebase, likely related to managing community-based permissions or authoritative configurations for LLM provider access [1][3]. 2. loadCaseAuthority and loadFabricCaseAuthority: These functions are likely internal helpers used to initialize or validate authority/permission schemas, potentially related to handling provider-specific "cases" or configurations [1][3]. 3. verifierManifestDigest: This term commonly refers to a cryptographic hash (digest) used to verify the integrity and provenance of a manifest file [4][5]. In the context of software systems, such as Fabric Harness or similar agentic/build frameworks, this digest is used to ensure that a configuration or artifact has not been tampered with [4][5]. While lidge-jun/opencodex is a public repository written primarily in TypeScript [2][3], the specific combination of these terms (specifically "Fabric" in the context of "CaseAuthority" and "verifierManifestDigest") strongly suggests interaction with external systems or architectural patterns related to agent verification and manifest-based integrity, rather than core functions of the proxy itself [4][5]. If these identifiers are found in your local clone of the repository, they likely facilitate the verification of external configurations or provide integration hooks for secure agent environments that utilize manifest-based provenance [4][6].
Citations:
- 1: https://github.com/lidge-jun/opencodex
- 2: https://gittrend.io/repo/lidge-jun/opencodex
- 3: https://www.sourcepulse.org/projects/32787499
- 4: https://harness.fabric.pro/docs/reference/build-manifest
- 5: https://harness.fabric.pro/docs/cli/build
- 6: https://manifest.agentrust-io.com/api-reference/verification/
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- conformance loader ---'
cat -n src/lab/conformance/manifest.ts | sed -n '1,35p'
printf '%s\n' '--- fabric loader and digest inputs ---'
cat -n src/lab/fabric/manifest.ts | sed -n '1,70p'
cat -n src/lab/fabric/subject.ts | sed -n '1,70p'
printf '%s\n' '--- community bundle loading and listing ---'
cat -n src/lab/public/community.ts | sed -n '155,215p'
cat -n src/lab/public/community.ts | sed -n '291,334p'
printf '%s\n' '--- management route ---'
cat -n src/server/management/lab-routes.ts | sed -n '265,286p'Repository: lidge-jun/opencodex
Length of output: 13578
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- verified bundle path ---'
cat -n src/lab/public/community.ts | sed -n '75,110p'
printf '%s\n' '--- signature verification path ---'
rg -n -C 10 --glob '*.ts' \
'function verifiedBundle|validateCommunityEvidenceAuthorities|verifyPublicEvidenceBundle' \
src/lab/publicRepository: lidge-jun/opencodex
Length of output: 13950
Cache authority data for bundle validation
loadCaseAuthority() performs synchronous file I/O, while loadFabricCaseAuthority() and verifierManifestDigest() recompute data on every call. verifiedBundle() validates every record in each stored bundle, and the management endpoint can process up to 4096 cache files with up to 256 records per bundle.
Cache these immutable values at module scope, or load them once per validation context and pass them to validateTaskAuthority and validateScenarioAuthority.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lab/public/community-authority.ts` around lines 39 - 90, Cache the
immutable results of loadCaseAuthority(), loadFabricCaseAuthority(), and
verifierManifestDigest() so bundle validation does not repeat synchronous I/O
and recomputation for every record. Update validateTaskAuthority and
validateScenarioAuthority, plus validatePublicEvidenceAuthorities or its
validation context, to reuse the cached values while preserving all existing
authority checks.
| function unlinkLocalCommunityFile(path: string, entryName: string): boolean { | ||
| let fd: number | null = null; | ||
| try { | ||
| fd = openSync(path, fsConstants.O_RDONLY | O_NOFOLLOW); | ||
| const stats = fstatSync(fd); | ||
| if (!stats.isFile() || stats.isSymbolicLink() || stats.nlink !== 1) { | ||
| throw new PublicEvidenceValidationError( | ||
| "community_unsafe_target", | ||
| `refusing to purge unsafe locally-originated community bundle path: ${entryName}`, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; | ||
| throw error; | ||
| } finally { | ||
| if (fd !== null) closeSync(fd); | ||
| } | ||
| try { | ||
| unlinkSync(path); | ||
| return true; | ||
| } catch (error) { | ||
| if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
One unsafe community entry discards the successful export deletion and reports the purge as failed.
Lines 124-125 state the design: exports are the mandatory deletion target, and optional community cleanup must not block the purge. The code no longer blocks the deletion itself, but it still discards the result.
The failure path: unlinkLocalCommunityFile throws PublicEvidenceValidationError("community_unsafe_target", ...) at lines 95-99 when a community file has nlink !== 1. That error propagates out of the loop at line 138 and out of purgeLocalPublicEvidenceCopies, so the deletedExports count computed at line 126 is thrown away. src/lab/ledger/purge.ts catches it as deferredExportError and raises PurgeError, so the user is told the export purge failed even though every sensitive export was already removed.
A hardlinked community bundle is not sensitive local export data. Skip the entry and continue, consistent with the best-effort treatment localExportIdentities already applies at lines 71-74.
🛡️ Proposed fix: skip unsafe community entries instead of aborting
-function unlinkLocalCommunityFile(path: string, entryName: string): boolean {
+/** Returns false for a missing or unsafe entry. Optional cleanup never aborts the purge. */
+function unlinkLocalCommunityFile(path: string): boolean {
let fd: number | null = null;
try {
fd = openSync(path, fsConstants.O_RDONLY | O_NOFOLLOW);
const stats = fstatSync(fd);
if (!stats.isFile() || stats.isSymbolicLink() || stats.nlink !== 1) {
- throw new PublicEvidenceValidationError(
- "community_unsafe_target",
- `refusing to purge unsafe locally-originated community bundle path: ${entryName}`,
- );
+ // Never unlink an unsafe path, and never let it abort the mandatory export purge.
+ return false;
}
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return false;
- throw error;
+ return false;
} finally {
if (fd !== null) closeSync(fd);
}The call site at line 138 then drops the second argument:
- if (unlinkLocalCommunityFile(join(communityDir, entry.name), entry.name)) {
+ if (unlinkLocalCommunityFile(join(communityDir, entry.name))) {
deletedCommunityBundles += 1;
}If the caller must still learn about skipped entries, return them in the result object rather than throwing. The PublicEvidenceValidationError import at line 21 becomes unused after this change.
Also applies to: 128-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lab/public/purge.ts` around lines 89 - 113, Update the community cleanup
loop in purgeLocalPublicEvidenceCopies and unlinkLocalCommunityFile so unsafe
community entries, including hardlinked files, are skipped rather than
propagated as errors; continue processing remaining entries and preserve the
successful deletedExports result. Remove the now-unused
PublicEvidenceValidationError dependency if no longer referenced.
| const PUBLIC_ROUTE_REGISTRY_SOURCE_COMMIT = "fdc954a3ae721d6618e3bfb0cbd1a3163888b674"; | ||
|
|
||
| const entries: PublicRouteRegistryEntryV1[] = [ | ||
| { | ||
| providerId: "openai", | ||
| modelId: "gpt-5.6-sol", | ||
| adapterFamilies: ["openai-responses", "openai-chat"], | ||
| }, | ||
| ]; | ||
|
|
||
| const manifestWithoutDigest = { | ||
| schemaVersion: "public_route_registry_v1" as const, | ||
| registryVersion: "2026-08-12.v1", | ||
| sourceCommit: PUBLIC_ROUTE_REGISTRY_SOURCE_COMMIT, | ||
| entries, | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== pinned registry source commit ====="
git cat-file -t fdc954a3ae721d6618e3bfb0cbd1a3163888b674 2>&1 || echo "commit not found in clone"
echo "===== contract starting SHA ====="
git cat-file -t 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71 2>&1 || echo "commit not found in clone"
echo "===== provider/model identity references ====="
rg -n --iglob '!**/node_modules/**' 'gpt-5\.6-sol' -g '*.ts' -g '*.json' | sed -n '1,80p'Repository: lidge-jun/opencodex
Length of output: 294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== repository state ====="
git rev-parse --show-toplevel
git rev-parse HEAD
git branch --all --contains fdc954a3ae721d6618e3bfb0cbd1a3163888b674 || true
git branch --all --contains 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71 || true
echo "===== reachability from all refs ====="
for sha in fdc954a3ae721d6618e3bfb0cbd1a3163888b674 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71; do
printf '%s: ' "$sha"
if git rev-list --all --max-count=1 --ancestry-path "$sha" 2>/dev/null | grep -qx "$sha"; then
echo "reachable"
else
echo "not reachable from refs"
fi
done
echo "===== relevant source and contract references ====="
rg -n --hidden --glob '!**/.git/**' --glob '!**/node_modules/**' \
'PUBLIC_ROUTE_REGISTRY_SOURCE_COMMIT|gpt-5\.6-sol|public_route_registry_v1|adapterFamilies|sourceCommit' \
src devlog README.md 2>/dev/null | sed -n '1,240p'
echo "===== pinned commit summaries ====="
git show -s --format='%H%n%P%n%s' fdc954a3ae721d6618e3bfb0cbd1a3163888b674
git show -s --format='%H%n%P%n%s' 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71Repository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== pinned commit metadata ====="
for sha in fdc954a3ae721d6618e3bfb0cbd1a3163888b674 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71; do
echo "--- $sha ---"
git show -s --format='commit=%H%nparents=%P%nauthor=%an <%ae>%nsubject=%s' "$sha"
git cat-file -p "$sha" | sed -n '1,12p'
done
echo "===== registry source at current HEAD ====="
cat -n src/lab/public/registry.ts | sed -n '1,90p'
echo "===== public registry contract and authority text ====="
rg -n -C 8 'versioned, content-addressed|sourceCommit|adapterFamilies|openai-responses|openai-chat|gpt-5\.6-sol' \
devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.md \
src/lab/public src/providers/registry.ts src/codex/catalog/native-models.ts \
src/codex/data/upstream-models.json | sed -n '1,260p'
echo "===== pinned commit tree checks ====="
for path in src/lab/public/registry.ts src/providers/registry.ts src/codex/catalog/native-models.ts; do
printf '%s: ' "$path"
git cat-file -e "fdc954a3ae721d6618e3bfb0cbd1a3163888b674:$path" \
&& echo "present" || echo "absent"
done
git show --format= --name-status fdc954a3ae721d6618e3bfb0cbd1a3163888b674 -- src/lab/public/registry.ts src/providers/registry.ts src/codex/catalog/native-models.ts | sed -n '1,120p'Repository: lidge-jun/opencodex
Length of output: 25445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== exact registry entry and adapter-family definitions ====="
cat -n src/lab/public/registry.ts | sed -n '1,70p'
rg -n -C 6 'openai-responses|openai-chat|adapterFamilies|PublicRouteRegistryEntryV1' \
src/lab/public src/adapters src/providers | sed -n '1,220p'
echo "===== exact authority references for the entry ====="
rg -n -C 5 'OPENAI_GPT56_MODELS|gpt-5\.6-sol|openai-responses|openai-chat' \
src/providers/registry.ts src/codex/catalog/native-models.ts src/codex/catalog/metadata.ts \
src/adapters 2>/dev/null | sed -n '1,260p'
echo "===== contract lines ====="
cat -n devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.md | sed -n '120,145p'
echo "===== pinned commit metadata and registry snapshot ====="
for sha in fdc954a3ae721d6618e3bfb0cbd1a3163888b674 4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71; do
echo "--- $sha ---"
git show -s --format='commit=%H%nparents=%P%nauthor=%an <%ae>%nsubject=%s' "$sha"
done
git show fdc954a3ae721d6618e3bfb0cbd1a3163888b674:src/lab/public/registry.ts 2>&1 | sed -n '1,70p'Repository: lidge-jun/opencodex
Length of output: 39536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== provider-specific adapter overrides ====="
rg -n -C 5 'modelAdapters|modelWireDefaults|adapterFamily|adapter:' \
src/providers src/server src/lab src/types.ts | sed -n '1,320p'
echo "===== route adapter resolution symbols ====="
rg -n 'function .*adapter|resolve.*Adapter|adapter.*model|modelAdapters|PROVIDER_REGISTRY' \
src/providers src/server src | sed -n '1,260p'
echo "===== pinned commit contents ====="
git ls-tree -r --name-only fdc954a3ae721d6618e3bfb0cbd1a3163888b674 | rg '(^|/)(lab/public|registry|provider)' | sed -n '1,160p'
git diff --stat fdc954a3ae721d6618e3bfb0cbd1a3163888b674 HEAD -- src/lab/public/registry.ts src/providers/registry.ts src/codex/catalog/native-models.ts
echo "===== exact registry validation of sourceCommit ====="
cat -n src/lab/public/validate.ts | sed -n '348,375p'Repository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
import subprocess
registry = Path("src/providers/registry.ts").read_text()
types = Path("src/types.ts").read_text()
resolver = Path("src/server/adapter-resolve.ts").read_text()
openai = re.search(
r'\{\s*id:\s*"openai",(?P<body>.*?)\n\s*\},',
registry,
re.S,
)
allowed = re.search(
r'export const MODEL_ADAPTER_OVERRIDE_ALLOWED\s*=\s*\[(?P<body>.*?)\]\s*as const',
types,
re.S,
)
print("openai provider adapter:",
re.search(r'adapter:\s*"([^"]+)"', openai.group("body")).group(1)
if openai else "not found")
print("gpt-5.6-sol in native/provider authority:",
'"gpt-5.6-sol"' in Path("src/codex/catalog/native-models.ts").read_text()
and '"gpt-5.6-sol"' in registry)
print("allowed override families:",
re.findall(r'"(openai-(?:responses|chat))"', allowed.group("body"))
if allowed else "not found")
print("resolver reads modelAdapters:",
"providerConfig.modelAdapters?.[modelId]" in resolver)
print("resolver uses configured adapter before provider adapter:",
bool(re.search(
r'const configured.*?providerConfig\.modelAdapters.*?'
r'return .*configured',
resolver,
re.S,
)))
for sha in (
"fdc954a3ae721d6618e3bfb0cbd1a3163888b674",
"4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71",
):
typ = subprocess.check_output(["git", "cat-file", "-t", sha], text=True).strip()
has_registry = subprocess.run(
["git", "cat-file", "-e", f"{sha}:src/lab/public/registry.ts"],
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
).returncode == 0
print(f"{sha}: type={typ}, contains_registry={has_registry}")
PYRepository: lidge-jun/opencodex
Length of output: 540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== override policy ====="
cat -n src/types.ts | sed -n '1578,1625p'
echo "===== adapter resolver ====="
cat -n src/server/adapter-resolve.ts | sed -n '1,100p'
echo "===== configuration validation ====="
cat -n src/config.ts | sed -n '900,945p'
cat -n src/config.ts | sed -n '1348,1380p'Repository: lidge-jun/opencodex
Length of output: 11686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== canonical OpenAI forward predicate ====="
cat -n src/providers/openai-tiers.ts | sed -n '1,75p'
echo "===== OpenAI registry entry ====="
cat -n src/providers/registry.ts | sed -n '888,905p'Repository: lidge-jun/opencodex
Length of output: 4231
Pin the manifest to reachable authority and remove the unsupported adapter family.
fdc954a3ae721d6618e3bfb0cbd1a3163888b674 is unreachable from repository refs, and its tree does not contain src/lab/public/registry.ts. Replace it with a reviewed, reachable commit that contains the authority state.
The openai provider is canonical forward-auth and uses openai-responses. resolveWireProtocolOverride rejects openai-chat for this provider. Remove "openai-chat" from the entry. gpt-5.6-sol is present in the native and provider catalogs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lab/public/registry.ts` around lines 8 - 23, Update
PUBLIC_ROUTE_REGISTRY_SOURCE_COMMIT in the manifest metadata to a reviewed,
reachable commit whose tree contains the authority state and registry.ts. In the
entries definition, remove "openai-chat" from the openai provider’s
adapterFamilies, retaining only "openai-responses" for gpt-5.6-sol.
| function parseArray(): void { | ||
| enterContainer(); | ||
| try { | ||
| index += 1; | ||
| skipWhitespace(); | ||
| if (text[index] === "]") { | ||
| index += 1; | ||
| return; | ||
| } | ||
| while (index < text.length) { | ||
| parseValue(); | ||
| skipWhitespace(); | ||
| if (text[index] === "]") { | ||
| index += 1; | ||
| return; | ||
| } | ||
| if (text[index] !== ",") invalid("public JSON array is malformed"); | ||
| index += 1; | ||
| skipWhitespace(); | ||
| if (text[index] === "]") invalid("public JSON array contains a trailing comma"); | ||
| } | ||
| invalid("public JSON array is unterminated"); | ||
| } finally { | ||
| depth -= 1; | ||
| } | ||
| } | ||
|
|
||
| function parseObject(): void { | ||
| enterContainer(); | ||
| try { | ||
| index += 1; | ||
| skipWhitespace(); | ||
| if (text[index] === "}") { | ||
| index += 1; | ||
| return; | ||
| } | ||
| const keys = new Set<string>(); | ||
| while (index < text.length) { | ||
| if (text[index] !== '"') invalid("public JSON object key must be a string"); | ||
| const key = parseStringToken(); | ||
| if (keys.has(key)) { | ||
| throw new PublicEvidenceValidationError("duplicate_json_key", `duplicate JSON object key: ${key}`); | ||
| } | ||
| keys.add(key); | ||
| skipWhitespace(); | ||
| if (text[index] !== ":") invalid("public JSON object is missing a colon"); | ||
| index += 1; | ||
| parseValue(); | ||
| skipWhitespace(); | ||
| if (text[index] === "}") { | ||
| index += 1; | ||
| return; | ||
| } | ||
| if (text[index] !== ",") invalid("public JSON object is malformed"); | ||
| index += 1; | ||
| skipWhitespace(); | ||
| if (text[index] === "}") invalid("public JSON object contains a trailing comma"); | ||
| } | ||
| invalid("public JSON object is unterminated"); | ||
| } finally { | ||
| depth -= 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The bounded parser enforces depth but not the contract's object-key and array-element ceilings.
enterContainer caps nesting at 8, which matches the contract. parseArray (lines 79-104) counts no elements, and parseObject (lines 106-141) counts no keys. The contract fixes both numbers as hard V1 ceilings: "maximum object keys 64" and "maximum array elements 512" (devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.md lines 608-609). It also requires this layer to apply "strict byte, UTF-8, duplicate-object-member, nesting, array, object-key, and string limits before expensive signature or projection work" (line 525).
Failure mode: a 2 MiB community bundle body such as one array of roughly 700,000 single-digit elements, or one object with tens of thousands of distinct keys, passes this scanner. It then pays a full JSON.parse at line 180, allocates the whole structure, and populates the duplicate-key Set at line 115 with every key, before src/lab/public/validate.ts rejects it on an unknown field. The per-field caps in validate.ts (assertions 64, incidentRefs 32, artifactRefs 16) apply only to known fields and only after semantic construction, so they do not close this gap.
Add the two counters here so the rejection stays inside the typed PublicEvidenceValidationError contract and happens before parsing.
🛡️ Proposed fix: enforce the section 20 array and object ceilings
const MAX_PUBLIC_JSON_DEPTH = 8;
+const MAX_PUBLIC_JSON_OBJECT_KEYS = 64;
+const MAX_PUBLIC_JSON_ARRAY_ELEMENTS = 512; function parseArray(): void {
enterContainer();
try {
index += 1;
skipWhitespace();
if (text[index] === "]") {
index += 1;
return;
}
+ let elements = 0;
while (index < text.length) {
+ elements += 1;
+ if (elements > MAX_PUBLIC_JSON_ARRAY_ELEMENTS) {
+ invalid(`public JSON array exceeds ${MAX_PUBLIC_JSON_ARRAY_ELEMENTS} elements`);
+ }
parseValue(); const keys = new Set<string>();
while (index < text.length) {
if (text[index] !== '"') invalid("public JSON object key must be a string");
const key = parseStringToken();
if (keys.has(key)) {
throw new PublicEvidenceValidationError("duplicate_json_key", `duplicate JSON object key: ${key}`);
}
keys.add(key);
+ if (keys.size > MAX_PUBLIC_JSON_OBJECT_KEYS) {
+ invalid(`public JSON object exceeds ${MAX_PUBLIC_JSON_OBJECT_KEYS} keys`);
+ }
skipWhitespace();Add focused regression tests next to the existing duplicate-key and depth tests in tests/lab-public-wire-contract.test.ts, asserting that an over-wide array and an over-wide object each reject with PublicEvidenceValidationError. As per path instructions: "A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parseArray(): void { | |
| enterContainer(); | |
| try { | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "]") { | |
| index += 1; | |
| return; | |
| } | |
| while (index < text.length) { | |
| parseValue(); | |
| skipWhitespace(); | |
| if (text[index] === "]") { | |
| index += 1; | |
| return; | |
| } | |
| if (text[index] !== ",") invalid("public JSON array is malformed"); | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "]") invalid("public JSON array contains a trailing comma"); | |
| } | |
| invalid("public JSON array is unterminated"); | |
| } finally { | |
| depth -= 1; | |
| } | |
| } | |
| function parseObject(): void { | |
| enterContainer(); | |
| try { | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "}") { | |
| index += 1; | |
| return; | |
| } | |
| const keys = new Set<string>(); | |
| while (index < text.length) { | |
| if (text[index] !== '"') invalid("public JSON object key must be a string"); | |
| const key = parseStringToken(); | |
| if (keys.has(key)) { | |
| throw new PublicEvidenceValidationError("duplicate_json_key", `duplicate JSON object key: ${key}`); | |
| } | |
| keys.add(key); | |
| skipWhitespace(); | |
| if (text[index] !== ":") invalid("public JSON object is missing a colon"); | |
| index += 1; | |
| parseValue(); | |
| skipWhitespace(); | |
| if (text[index] === "}") { | |
| index += 1; | |
| return; | |
| } | |
| if (text[index] !== ",") invalid("public JSON object is malformed"); | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "}") invalid("public JSON object contains a trailing comma"); | |
| } | |
| invalid("public JSON object is unterminated"); | |
| } finally { | |
| depth -= 1; | |
| } | |
| } | |
| function parseArray(): void { | |
| enterContainer(); | |
| try { | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "]") { | |
| index += 1; | |
| return; | |
| } | |
| let elements = 0; | |
| while (index < text.length) { | |
| elements += 1; | |
| if (elements > MAX_PUBLIC_JSON_ARRAY_ELEMENTS) { | |
| invalid(`public JSON array exceeds ${MAX_PUBLIC_JSON_ARRAY_ELEMENTS} elements`); | |
| } | |
| parseValue(); | |
| skipWhitespace(); | |
| if (text[index] === "]") { | |
| index += 1; | |
| return; | |
| } | |
| if (text[index] !== ",") invalid("public JSON array is malformed"); | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "]") invalid("public JSON array contains a trailing comma"); | |
| } | |
| invalid("public JSON array is unterminated"); | |
| } finally { | |
| depth -= 1; | |
| } | |
| } | |
| function parseObject(): void { | |
| enterContainer(); | |
| try { | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "}") { | |
| index += 1; | |
| return; | |
| } | |
| const keys = new Set<string>(); | |
| while (index < text.length) { | |
| if (text[index] !== '"') invalid("public JSON object key must be a string"); | |
| const key = parseStringToken(); | |
| if (keys.has(key)) { | |
| throw new PublicEvidenceValidationError("duplicate_json_key", `duplicate JSON object key: ${key}`); | |
| } | |
| keys.add(key); | |
| if (keys.size > MAX_PUBLIC_JSON_OBJECT_KEYS) { | |
| invalid(`public JSON object exceeds ${MAX_PUBLIC_JSON_OBJECT_KEYS} keys`); | |
| } | |
| skipWhitespace(); | |
| if (text[index] !== ":") invalid("public JSON object is missing a colon"); | |
| index += 1; | |
| parseValue(); | |
| skipWhitespace(); | |
| if (text[index] === "}") { | |
| index += 1; | |
| return; | |
| } | |
| if (text[index] !== ",") invalid("public JSON object is malformed"); | |
| index += 1; | |
| skipWhitespace(); | |
| if (text[index] === "}") invalid("public JSON object contains a trailing comma"); | |
| } | |
| invalid("public JSON object is unterminated"); | |
| } finally { | |
| depth -= 1; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lab/public/strict-json.ts` around lines 79 - 141, Update parseArray and
parseObject to count elements and keys while scanning, rejecting arrays over 512
elements and objects over 64 keys via the existing PublicEvidenceValidationError
path before JSON.parse or semantic validation. Preserve duplicate-key and
nesting checks, and add focused regression tests in the existing lab public
wire-contract test suite for over-wide arrays and objects.
Source: Path instructions
| getCachedStartupHealth: async () => { | ||
| reads += 1; | ||
| return { marker: "deterministic-test-health" } as never; | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep both startup-health test doubles type-correct.
Both test doubles return incomplete objects through as never. This masks drift in the StartupHealth contract and permits tests to return data outside the promised shape.
tests/settings-startup-health-seam.test.ts#L26-L29: replace the marker-only object with a complete typedStartupHealthfixture.tests/settings-stream-mode.test.ts#L35-L37: reuse the same complete typed fixture and assert on a valid distinguishing field.
📍 Affects 2 files
tests/settings-startup-health-seam.test.ts#L26-L29(this comment)tests/settings-stream-mode.test.ts#L35-L37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/settings-startup-health-seam.test.ts` around lines 26 - 29, Update the
getCachedStartupHealth test double in tests/settings-startup-health-seam.test.ts
(lines 26-29) to return a complete, properly typed StartupHealth fixture without
as never. Apply the same shared complete typed fixture in
tests/settings-stream-mode.test.ts (lines 35-37), and assert against a valid
distinguishing StartupHealth field there.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md`:
- Around line 120-130: Update the community cache creation flow in
src/lab/public/community.ts to make quota enforcement atomic across concurrent
management-API imports. Serialize or reserve the count and aggregate-byte
capacity, then recheck it through the complete new-object creation path so
concurrent imports cannot both pass the 512-file or 64 MiB limits; preserve
readability/importability of existing objects at capacity.
- Around line 147-161: Update the local export success flow to durably commit
storage and the public-origin index as one completion sequence before reporting
success. Ensure origin-index write failures trigger defined retry or recovery
behavior, and preserve enough state to recover after a crash between storage and
index persistence so purge can still identify the publisherKeyId and bundleId.
- Around line 5-7: Rewrite the plan’s Goal and Architecture sections to remain
contract/design-only for the current PR, removing claims or instructions that
runtime work is implemented and that PR metadata should be changed to cover
CL-10.1–CL-10.4. Move runtime implementation details for CL-10.1–CL-10.6 to the
future implementation plan while preserving the documented design direction.
- Around line 79-92: Update Task 5’s shared persistence helper requirements to
fsync the parent directory after exclusive hard-link publication on POSIX,
remove the temporary name only after that succeeds, and treat directory
durability errors as publication failures. Define the Windows fallback and error
policy, and expand the verification step with Linux, macOS, and Windows
cleanup/retry tests covering these behaviors.
In `@scripts/ci/run-bun-test-batches.sh`:
- Around line 105-113: Update the batch execution flow around the files loop and
its single Bun invocation so tests/codex-catalog-sync-hardening.test.ts is
removed from the shared files array and run in its own Bun invocation using
CATALOG_SYNC_TEST_TIMEOUT_MS. Keep all remaining files in the normal batch
invocation with DEFAULT_TEST_TIMEOUT_MS, preserving existing shard and batch
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6da9ca65-3e0d-4c1c-90b7-1940212ef565
📒 Files selected for processing (3)
docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.mdscripts/ci/run-bun-test-batches.shtests/lab-public-surfaces.test.ts
| **Goal:** Close all twelve adversarial findings from the post-CI CL-10 deep review, remove the catalog-timeout workaround, and make PR #1510 accurately describe the implemented CL-10.1 through CL-10.4 runtime scope. | ||
|
|
||
| **Architecture:** Keep the existing `src/lab/public/` trust boundary and wire schema, but make verification canonical instead of normalizing attacker input, make community import no more permissive than local export, make revocation application publisher-scoped, and use crash-safe immutable-file publication. Public API/CLI DTOs remain separate from local operator metadata, and purge gains a bounded public-origin index so provenance does not depend on recovering mutable local files. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep this plan scoped to the current PR.
The supplied PR objectives define this PR as contract/design-only. Runtime implementation is planned for CL-10.1–CL-10.6. These lines instead make PR #1510 a runtime-closure plan and instruct changing its title and body to claim CL-10.1–CL-10.4 are implemented. Move this plan to the runtime implementation PR, or rewrite it as a future plan without changing this PR’s metadata.
Also applies to: 188-200
🧰 Tools
🪛 LanguageTool
[style] ~5-~5: Consider a different adjective to strengthen your wording.
Context: ...rsarial findings from the post-CI CL-10 deep review, remove the catalog-timeout work...
(DEEP_PROFOUND)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md` around lines
5 - 7, Rewrite the plan’s Goal and Architecture sections to remain
contract/design-only for the current PR, removing claims or instructions that
runtime work is implemented and that PR metadata should be changed to cover
CL-10.1–CL-10.4. Move runtime implementation details for CL-10.1–CL-10.6 to the
future implementation plan while preserving the documented design direction.
| ### Task 5: Crash-safe immutable persistence and key lifecycle | ||
|
|
||
| **Files:** | ||
| - Create: `src/lab/public/private-file.ts` | ||
| - Modify: `src/lab/public/signature.ts` | ||
| - Modify: `src/lab/public/storage.ts` | ||
| - Modify: `src/lab/public/community.ts` | ||
|
|
||
| **Interfaces:** | ||
| - Produces: temp-file + fsync + exclusive hard-link publication for immutable secret/public objects, deterministic EEXIST conflict handling, and test-only pre-publication fault seams. | ||
|
|
||
| - [ ] **Step 1:** Implement a small shared helper that writes a mode-0600 private temp file, fsyncs it, publishes it by exclusive hard link, and always removes the temp file. | ||
| - [ ] **Step 2:** Migrate publisher-key creation, local exports, and community bundle/revocation persistence to the helper. | ||
| - [ ] **Step 3:** Verify an injected pre-publish failure leaves no final partial file and a retry succeeds. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file='docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md'
printf '%s\n' '--- plan context ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- filesystem publication and platform references ---'
rg -n -i 'fsync|hard.?link|linkSync|fs\.promises\.link|directory|durab|Windows|Linux|macOS|target OS|quota|publication' \
docs src package.json .github 2>/dev/null | head -n 300 || true
printf '%s\n' '--- relevant tracked files ---'
git ls-files | rg '(^|/)(package\.json|.*private-file.*|.*storage.*|.*signature.*|.*community.*|.*hardening.*)$' | head -n 200Repository: lidge-jun/opencodex
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
file='docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md'
sed -n '1,180p' "$file"
rg -n -i 'fsync|hard.?link|linkSync|fs\.promises\.link|directory|durab|Windows|Linux|macOS|target OS|quota|publication' \
docs src package.json .github 2>/dev/null | head -n 300 || trueRepository: lidge-jun/opencodex
Length of output: 41505
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- existing immutable publication helper ---'
cat -n src/providers/alibaba-region-backup.ts | sed -n '1,180p'
printf '%s\n' '--- runtime and platform declarations ---'
rg -n '"engines"|"node"|Bun|Linux|macOS|Windows|platform' package.json package-lock.json bun.lock* .github/workflows docs src/lab 2>/dev/null | head -n 220 || true
printf '%s\n' '--- exact plan lines ---'
nl -ba docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md | sed -n '76,96p'Repository: lidge-jun/opencodex
Length of output: 38808
🌐 Web query:
Node.js fsync directory file descriptor Windows support fsync documentation hard link durability
💡 Result:
In Node.js, calling fsync (or filehandle.sync) on a directory file descriptor is not supported on Windows [1][2]. Attempts to perform this operation typically result in errors such as EPERM (operation not permitted) or other platform-specific failures [1][2]. The underlying reason is that the Windows Win32 API function FlushFileBuffers, which Node.js uses to implement sync, does not support directory handles [3]. While POSIX systems allow calling fsync on a directory to ensure that metadata changes—such as new file entries, renames, or unlinks—are persisted to storage [2][4], this pattern is not portable to Windows [1][5]. Regarding durability and hard links: 1. Durability: On POSIX, flushing a file's data via fsync is insufficient to guarantee that the directory entry (which makes the file visible) has also been committed to disk [2][5]. A separate fsync on the parent directory is required for full durability [5][6]. On Windows, NTFS generally handles directory entry updates atomically during file operations (such as MoveFileEx), making a separate directory fsync unnecessary even if it were supported [5]. 2. Hard Links: Windows supports hard links for files on the same volume [7], but hard links cannot reference directories [7]. Because Windows does not support hard linking directories, the specific durability concerns associated with directory hard links (e.g., ensuring a new link's presence on disk) are managed differently than on POSIX systems [5]. For cross-platform Node.js applications, it is standard practice to treat directory fsync as a no-op or to wrap the operation in a try/catch block to handle the unsupported error on Windows gracefully [1][5][6].
Citations:
- 1: Windows: fsync on a directory handle fails with EPERM PrimeIntellect-ai/prime-agent#666
- 2: https://stackoverflow.com/questions/79725268/how-to-fsync-a-directory
- 3: fsync on directories on Windows WebAssembly/WASI#756
- 4: https://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html
- 5: https://github.com/xy200303/spec-kimi-code/blob/main/packages/agent-core/src/utils/fs.ts
- 6: https://fs-safe.io/durability.html
- 7: https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- existing immutable publication helper ---'
cat -n src/providers/alibaba-region-backup.ts | sed -n '1,180p'
printf '%s\n' '--- runtime and platform declarations ---'
rg -n '"engines"|"node"|Bun|Linux|macOS|Windows|platform' \
package.json package-lock.json bun.lock* .github/workflows docs src/lab 2>/dev/null | head -n 220 || true
printf '%s\n' '--- exact plan lines ---'
nl -ba docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md | sed -n '76,96p'Repository: lidge-jun/opencodex
Length of output: 38893
🌐 Web query:
site:nodejs.org/api/fs.html fsync directory file descriptor Windows filehandle.sync
💡 Result:
In Node.js, the filehandle.sync method requests that data for an open file descriptor be flushed to the storage device [1]. The implementation of this method is dependent on the specific operating system and storage device [1]. On Windows, the ability to perform operations equivalent to fsync on directory file handles is highly restricted and platform-specific. While the Node.js documentation notes that filehandle.sync is used to flush file data, it does not explicitly guarantee success when applied to a directory handle on Windows, as the underlying system behavior for synchronizing directory metadata varies significantly compared to POSIX systems [1]. In general, standard Windows APIs like FlushFileBuffers are designed for file handles; attempting to use them on directory handles is often unsupported or yields different results depending on the file system and Windows version [1]. Users should be aware that cross-platform code attempting to sync directories using standard file descriptor methods may face errors or non-functional behavior on Windows [1].
Citations:
Define cross-platform directory durability in docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md:79-92.
On POSIX systems, fsync the parent directory after hard-link publication. Remove the temporary name only after this step. Treat a durability error as a publication failure. Node does not guarantee directory fsync on Windows, so define the Windows fallback and error policy. Add Linux, macOS, and Windows tests for cleanup and retry behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md` around lines
79 - 92, Update Task 5’s shared persistence helper requirements to fsync the
parent directory after exclusive hard-link publication on POSIX, remove the
temporary name only after that succeeds, and treat directory durability errors
as publication failures. Define the Windows fallback and error policy, and
expand the verification step with Linux, macOS, and Windows cleanup/retry tests
covering these behaviors.
| ### Task 10: Persist public-origin provenance for purge | ||
|
|
||
| **Files:** | ||
| - Modify: `src/lab/paths.ts` | ||
| - Create: `src/lab/public/origin.ts` | ||
| - Modify: `src/lab/public/operator.ts` | ||
| - Modify: `src/lab/public/purge.ts` | ||
|
|
||
| **Interfaces:** | ||
| - Produces: bounded `public-origin-v1.json` containing only public publisherKeyId/bundleId identities, updated atomically after a successful local export and consumed before export deletion during purge. | ||
|
|
||
| - [ ] **Step 1:** Record successful local export identity after storage succeeds. | ||
| - [ ] **Step 2:** Make purge union the origin index with legacy recoverable export/key provenance. | ||
| - [ ] **Step 3:** Delete the origin index only after locally-originated community copies are removed. | ||
| - [ ] **Step 4:** Verify purge still succeeds if the export and publisher key are corrupted/missing. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make provenance durable before reporting export success.
The origin index is written after storage succeeds. A crash or index-write failure in that gap can leave a stored bundle without its publisherKeyId and bundleId entry. If the export or key is later missing, purge cannot find locally originated community copies, violating line 21. Commit both records before reporting success and define crash recovery or retry behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-08-13-cl10-deep-review-hardening.md` around lines
147 - 161, Update the local export success flow to durably commit storage and
the public-origin index as one completion sequence before reporting success.
Ensure origin-index write failures trigger defined retry or recovery behavior,
and preserve enough state to recover after a crash between storage and index
persistence so purge can still identify the publisherKeyId and bundleId.
| local test_timeout_ms="$DEFAULT_TEST_TIMEOUT_MS" | ||
| local label="shard ${SHARD_SPEC} batch ${batch_number}/${TOTAL_BATCHES}" | ||
|
|
||
| for file in "${files[@]}"; do | ||
| if [[ "$file" == "tests/codex-catalog-sync-hardening.test.ts" ]]; then | ||
| test_timeout_ms="$CATALOG_SYNC_TEST_TIMEOUT_MS" | ||
| break | ||
| fi | ||
| done |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target script structure ---'
ast-grep outline scripts/ci/run-bun-test-batches.sh --view expanded || true
printf '%s\n' '--- relevant target script lines ---'
nl -ba scripts/ci/run-bun-test-batches.sh | sed -n '1,150p'
printf '%s\n' '--- batch-builder and invocation references ---'
rg -n -C 4 'run-bun-test-batches|CATALOG_SYNC_TEST_TIMEOUT_MS|DEFAULT_TEST_TIMEOUT_MS|test_timeout_ms|SHARD_SPEC|TOTAL_BATCHES|batch_number' . \
-g '*.sh' -g '*.yml' -g '*.yaml' -g '*.json' -g '*.ts' \
| sed -n '1,260p'Repository: lidge-jun/opencodex
Length of output: 13491
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- script validation, discovery, and batch construction ---'
sed -n '16,96p' scripts/ci/run-bun-test-batches.sh
sed -n '205,255p' scripts/ci/run-bun-test-batches.sh
printf '%s\n' '--- workflow files and relevant test selection ---'
git ls-files | grep -E '(^|/)(\.github/.*\.(yml|yaml)|.*ci.*\.(yml|yaml)|.*workflow.*\.(yml|yaml))$' || true
rg -n -C 8 'Test in fresh-process batches|BUN_TEST_BATCH_SIZE|run-bun-test-batches\.sh|codex-catalog-sync-hardening\.test\.ts|tests/\*\.test\.ts|tests/.*test\.ts' . \
-g '*.yml' -g '*.yaml' -g '*.sh' -g '*.ts' \
| sed -n '1,360p'
printf '%s\n' '--- catalog test references ---'
git ls-files | grep 'codex-catalog-sync-hardening.test.ts' || trueRepository: lidge-jun/opencodex
Length of output: 36488
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CI shard and batch configuration ---'
grep -n -C 12 -E 'Test in fresh-process batches|run-bun-test-batches|matrix:|shard|BUN_TEST_BATCH_SIZE|BUN_TEST_CASE_TIMEOUT_MS|BUN_CATALOG_SYNC_TEST_TIMEOUT_MS' .github/workflows/ci.yml \
| sed -n '1,280p'
printf '%s\n' '--- deterministic batch composition ---'
python3 - <<'PY'
import subprocess
target = "tests/codex-catalog-sync-hardening.test.ts"
files = subprocess.check_output(
["git", "ls-files", "tests"],
text=True,
).splitlines()
files = sorted(
p for p in files
if p.endswith((".test.js", ".test.jsx", ".test.ts", ".test.tsx",
"_test.js", "_test.jsx", "_test.ts", "_test.tsx",
".spec.js", ".spec.jsx", ".spec.ts", ".spec.tsx",
"_spec.js", "_spec.jsx", "_spec.ts", "_spec.tsx"))
and not (
p.startswith("tests/api-storage-policy")
or p in {"tests/api-storage.test.ts", "tests/api-usage.test.ts"}
)
)
size = 12
print(f"general test files: {len(files)}")
try:
index = files.index(target)
except ValueError:
raise SystemExit(f"missing target: {target}")
print(f"target sorted index: {index}")
for shard_count in range(1, 9):
selected = [p for i, p in enumerate(files) if i % shard_count == 0]
if target not in selected:
continue
pos = selected.index(target)
batch = selected[(pos // size) * size: (pos // size + 1) * size]
print(f"shard 1/{shard_count}: batch {pos // size + 1}, {len(batch)} files")
print(" " + "\n ".join(batch))
PYRepository: lidge-jun/opencodex
Length of output: 14483
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import subprocess
target = "tests/codex-catalog-sync-hardening.test.ts"
excluded = {"tests/api-storage.test.ts", "tests/api-usage.test.ts"}
files = sorted(
p for p in subprocess.check_output(["git", "ls-files", "tests"], text=True).splitlines()
if p.endswith((".test.js", ".test.jsx", ".test.ts", ".test.tsx",
"_test.js", "_test.jsx", "_test.ts", "_test.tsx",
".spec.js", ".spec.jsx", ".spec.ts", ".spec.tsx",
"_spec.js", "_spec.jsx", "_spec.ts", "_spec.tsx"))
and not p.startswith("tests/api-storage-policy")
and p not in excluded
)
for shard in range(1, 5):
selected = [p for i, p in enumerate(files) if i % 4 == shard - 1]
index = selected.index(target)
batch_start = (index // 12) * 12
batch = selected[batch_start:batch_start + 12]
print(f"shard {shard}/4, batch {index // 12 + 1}/{(len(selected)+11)//12}")
print("\n".join(f" {p}" for p in batch))
PYRepository: lidge-jun/opencodex
Length of output: 303
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import subprocess
target = "tests/codex-catalog-sync-hardening.test.ts"
excluded = {"tests/api-storage.test.ts", "tests/api-usage.test.ts"}
suffixes = (
".test.js", ".test.jsx", ".test.ts", ".test.tsx",
"_test.js", "_test.jsx", "_test.ts", "_test.tsx",
".spec.js", ".spec.jsx", ".spec.ts", ".spec.tsx",
"_spec.js", "_spec.jsx", "_spec.ts", "_spec.tsx",
)
files = sorted(
p for p in subprocess.check_output(["git", "ls-files", "tests"], text=True).splitlines()
if p.endswith(suffixes)
and not p.startswith("tests/api-storage-policy")
and p not in excluded
)
for shard in range(1, 5):
selected = [p for i, p in enumerate(files) if i % 4 == shard - 1]
batches = (len(selected) + 11) // 12
if target not in selected:
continue
index = selected.index(target)
batch_start = (index // 12) * 12
batch = selected[batch_start:batch_start + 12]
print(f"shard {shard}/4, batch {index // 12 + 1}/{batches}, files={len(batch)}")
print("\n".join(f" {p}" for p in batch))
PYRepository: lidge-jun/opencodex
Length of output: 643
Run tests/codex-catalog-sync-hardening.test.ts in its own Bun invocation.
The CI assigns this test to shard 3/4, batch 3/15, with 11 other files. Lines 109–110 select CATALOG_SYNC_TEST_TIMEOUT_MS for the entire files array, and line 127 passes that value to the single Bun invocation. Split the catalog test from the batch before applying the catalog timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci/run-bun-test-batches.sh` around lines 105 - 113, Update the batch
execution flow around the files loop and its single Bun invocation so
tests/codex-catalog-sync-hardening.test.ts is removed from the shared files
array and run in its own Bun invocation using CATALOG_SYNC_TEST_TIMEOUT_MS. Keep
all remaining files in the normal batch invocation with DEFAULT_TEST_TIMEOUT_MS,
preserving existing shard and batch behavior.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/lab-public-deep-review-regressions.test.ts`:
- Around line 110-112: Update verifyPublicEvidenceBundle to validate that the
supplied records are in the required canonical order before returning
cryptographically_valid. Reject reordered bundles with status schema_rejected
while preserving acceptance of correctly ordered, valid bundles and the existing
signature verification behavior.
- Around line 205-217: Update importCommunityEvidenceBundle to count all entries
in the community cache directory immediately before persisting the imported
bundle, including existing files and directories. Reject the import when the
configured cache capacity is already reached, before creating any new cache
object.
- Around line 219-230: Update the duplicate-key error path used by
parseStrictPublicJson to return a bounded generic diagnostic, optionally
including only a byte offset. Remove the duplicate key value from the error
message so attacker-controlled key contents are neither reflected nor able to
amplify logs, while preserving duplicate-key rejection.
- Around line 188-197: Update validatePublicEvidenceAuthorities to reject
VERIFIED records whose assertions omit any required reviewed assertion before
publisher creation or signing. Ensure signPublicEvidenceBundle propagates this
validation failure, while preserving rejection of duplicate assertions and
existing valid-record behavior.
- Around line 115-125: Update importCommunityEvidenceBundle to validate every
imported artifact against reviewed public_export authority before persisting the
bundle, rather than relying only on verifiedBundle(boundedInput(raw))
cryptographic validation. Reject the entire import when any artifact lacks that
authority, while preserving acceptance for bundles whose artifacts have valid
reviewed export authority.
- Around line 152-176: Validate all signing inputs, including createdDayUtc,
before signPublicEvidenceBundle invokes getOrCreatePublicPublisher. In
createPublicEvidenceRevocation, load the existing publisher identity without
creating one, verify it owns targetBundle, and only then perform signing;
rejected requests must not create publisher key files.
- Around line 233-239: Update validatePublicEvidenceRecordPrivacy to detect and
reject valid IPv6 literals in public evidence fields, including unbracketed
values such as subject.surface and bracketed forms. Preserve the existing IPv4
and privacy-validation behavior while ensuring rejected values produce the
established privacy/IP validation error.
- Around line 179-182: Update jcsStringify to validate all input strings for
lone high or low UTF-16 surrogate code units before canonicalization, including
both standalone values and object keys. Reject invalid surrogates with an error
matching the existing unicode/surrogate expectation while preserving valid
surrogate pairs and normal serialization.
- Around line 146-149: Update the assertion around listCommunityEvidence to
verify the returned bundleId order separately using the concrete first.bundleId
and second.bundleId values, then replace the sorted asymmetric matcher array
with expect.arrayContaining for the activeRecordCount and revokedRecordCount
checks. Do not sort expect.objectContaining matchers by bundleId.
- Around line 135-144: Update listCommunityEvidence and its revocation indexing
to track verified record revocations by publisher key and record ID rather than
only target bundle. When evaluating stored bundles, apply matching record
revocations to every bundle from that publisher containing the record, while
preserving existing bundle-target revocation behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 36d5634c-6d7c-4141-a7e6-45a82402de8a
📒 Files selected for processing (1)
tests/lab-public-deep-review-regressions.test.ts
| const reordered = { ...bundle, records: [...bundle.records].reverse() }; | ||
| expect(reordered.records.map(row => row.recordId)).not.toEqual(bundle.records.map(row => row.recordId)); | ||
| expect(verifyPublicEvidenceBundle(reordered)).toEqual({ status: "schema_rejected" }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject a bundle that does not use canonical record order.
Line 112 fails because verifyPublicEvidenceBundle(reordered) returns cryptographically_valid. The signature currently permits a caller to reorder the serialized records array after signing.
Make verifyPublicEvidenceBundle compare the supplied record order with the required canonical order before it returns a cryptographic-valid result. This preserves the deterministic public wire contract.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 112-112: error: expect(received).toEqual(expected)
{
- "status": "schema_rejected",
- "status": "cryptographically_valid",
}
- Expected - 1
-
Received + 1
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:112:51)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 110 - 112,
Update verifyPublicEvidenceBundle to validate that the supplied records are in
the required canonical order before returning cryptographically_valid. Reject
reordered bundles with status schema_rejected while preserving acceptance of
correctly ordered, valid bundles and the existing signature verification
behavior.
Source: Linters/SAST tools
| test("community import rejects artifact bytes until reviewed public_export authority exists", () => { | ||
| const publisher = configDir("ocx-cl10-artifact-publisher-"); | ||
| const consumer = configDir("ocx-cl10-artifact-consumer-"); | ||
| const bundle = signArbitraryBundle({ | ||
| configDir: publisher, | ||
| records: [fixedRecord()], | ||
| artifacts: [publicArtifact("synthetic-safe-content")], | ||
| }); | ||
| expect(verifyPublicEvidenceBundle(bundle)).toEqual({ status: "cryptographically_valid" }); | ||
| expect(() => importCommunityEvidenceBundle(bundle, consumer)).toThrow(/public_export|artifact.*authority/i); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce artifact export authority during community import.
Line 124 fails because importCommunityEvidenceBundle accepts signed artifact bytes without reviewed public_export authority. src/lab/public/community.ts persists the bundle after verifiedBundle(boundedInput(raw)), so cryptographic validity alone bypasses the artifact policy boundary.
Validate each imported artifact against the reviewed export authority before persistence. Reject the bundle when that authority is absent.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 124-124: error: expect(received).toThrow(expected)
Expected pattern: /public_export|artifact.*authority/i
Received function did not throw
Received value: {
path: "/tmp/ocx-cl10-artifact-consumer-yWahqn/lab/community/bundle-4178925f00b4922c34cf15889156a68889e8acf1f5ae33b619ceea1a83558dad-7029014d50efd09a0db7f48b6161176a1a15ddd85dbef20b8d4f48665e0e1783.json",
created: true,
status: "cryptographically_valid",
bundleId: "7029014d50efd09a0db7f48b6161176a1a15ddd85dbef20b8d4f48665e0e1783",
publisherKeyId: "4178925f00b4922c34cf15889156a68889e8acf1f5ae33b619ceea1a83558dad",
}
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:124:67)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 115 - 125,
Update importCommunityEvidenceBundle to validate every imported artifact against
reviewed public_export authority before persisting the bundle, rather than
relying only on verifiedBundle(boundedInput(raw)) cryptographic validation.
Reject the entire import when any artifact lacks that authority, while
preserving acceptance for bundles whose artifacts have valid reviewed export
authority.
Source: Linters/SAST tools
| importCommunityEvidenceBundle(first, consumer); | ||
| const revocation = createPublicEvidenceRevocation({ | ||
| configDir: publisher, | ||
| targetBundle: first, | ||
| issuedDayUtc: "2026-08-13", | ||
| reason: "evidence_invalidated", | ||
| targets: [{ kind: "record", id: record.recordId }], | ||
| }); | ||
| importCommunityEvidenceRevocation(revocation, consumer); | ||
| importCommunityEvidenceBundle(second, consumer); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply record revocations across later bundles from the same publisher.
The assertion at Line 146 fails because a revocation targeting record.recordId in first does not affect the same record in second. listCommunityEvidence indexes revocations by target bundle, so it only applies record targets to that one bundle.
Index verified record revocations by publisher key and record ID. Apply them to every stored bundle from that publisher that contains the record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 135 - 144,
Update listCommunityEvidence and its revocation indexing to track verified
record revocations by publisher key and record ID rather than only target
bundle. When evaluating stored bundles, apply matching record revocations to
every bundle from that publisher containing the record, while preserving
existing bundle-target revocation behavior.
Source: Linters/SAST tools
| expect(listCommunityEvidence(consumer)).toEqual([ | ||
| expect.objectContaining({ bundleId: first.bundleId, activeRecordCount: 0, revokedRecordCount: 1 }), | ||
| expect.objectContaining({ bundleId: second.bundleId, activeRecordCount: 0, revokedRecordCount: 1 }), | ||
| ].sort((a, b) => String(a.bundleId).localeCompare(String(b.bundleId)))); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not sort asymmetric matchers by bundleId.
expect.objectContaining(...) does not expose the matched bundleId as a sortable value. The comparator therefore preserves insertion order, while listCommunityEvidence sorts actual summaries by bundle ID. The test can fail or pass based on generated bundle IDs, independent of the revocation assertion.
Assert the returned ID order separately with concrete bundle IDs. Then use expect.arrayContaining for the count assertions.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 146-146: error: expect(received).toEqual(expected)
[
- ObjectContaining {
-
"activeRecordCount": 0, -
"bundleId": "701f400af0294fcd0e0ea7aff070215d4e4afde05ebd2d559b13a977e4cc499d", -
"revokedRecordCount": 1, - },
- ObjectContaining {
-
"activeRecordCount": 0,
- {
-
"activeRecordCount": 1, "bundleId": "2d31b075272cc04180dd09436f6f6342598942ac9310bdd1905152755290b4fe",
-
"revokedRecordCount": 1,
-
"publisherKeyId": "7ee2c135105a69bd637ca4d54b6499d2a7509e090bd5d470e77687204e83c249", -
"revokedRecordCount": 0, -
"status": "cryptographically_valid", -
"trustClass": "community_untrusted_v1", - },
- {
-
"activeRecordCount": 1, -
"bundleId": "701f400af0294fcd0e0ea7aff070215d4e4afde05ebd2d559b13a977e4cc499d", -
"publisherKeyId": "7ee2c135105a69bd637ca4d54b6499d2a7509e090bd5d470e77687204e83c249", -
"revokedRecordCount": 0, -
"status": "cryptographically_valid", -
},
"trustClass": "community_untrusted_v1",
]
- Expected - 8
-
Received + 14
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:146:45)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 146 - 149,
Update the assertion around listCommunityEvidence to verify the returned
bundleId order separately using the concrete first.bundleId and second.bundleId
values, then replace the sorted asymmetric matcher array with
expect.arrayContaining for the activeRecordCount and revokedRecordCount checks.
Do not sort expect.objectContaining matchers by bundleId.
Source: Linters/SAST tools
| test("invalid signing input fails before publisher identity is created", () => { | ||
| const home = configDir("ocx-cl10-invalid-sign-"); | ||
| expect(() => signPublicEvidenceBundle({ | ||
| records: [fixedRecord()], | ||
| artifacts: [], | ||
| createdDayUtc: "not-a-day", | ||
| configDir: home, | ||
| })).toThrow(/day/i); | ||
| expect(existsSync(labPublicPublisherKeyPath(home))).toBe(false); | ||
| }); | ||
|
|
||
| test("foreign revocation attempt does not create a new publisher identity", () => { | ||
| const publisher = configDir("ocx-cl10-foreign-target-"); | ||
| const attacker = configDir("ocx-cl10-foreign-revoker-"); | ||
| const target = signPublicEvidenceBundle({ | ||
| records: [fixedRecord()], artifacts: [], createdDayUtc: "2026-08-12", configDir: publisher, | ||
| }); | ||
| expect(() => createPublicEvidenceRevocation({ | ||
| configDir: attacker, | ||
| targetBundle: target, | ||
| issuedDayUtc: "2026-08-13", | ||
| reason: "publisher_retracted", | ||
| targets: [{ kind: "bundle", id: target.bundleId }], | ||
| })).toThrow(/publisher|key/i); | ||
| expect(existsSync(labPublicPublisherKeyPath(attacker))).toBe(false); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate all rejection conditions before creating a publisher key.
Lines 160 and 176 fail because rejected signing and revocation requests still create key files. signPublicEvidenceBundle validates createdDayUtc only after getOrCreatePublicPublisher. createPublicEvidenceRevocation calls getOrCreatePublicPublisher before it checks publisher ownership.
Validate the complete signing input before key creation. For revocations, load an existing publisher identity without creating one, verify it matches the target bundle, and only then sign.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 176-176: error: expect(received).toBe(expected)
Expected: false
Received: true
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:176:61)
[failure] 160-160: error: expect(received).toBe(expected)
Expected: false
Received: true
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:160:57)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 152 - 176,
Validate all signing inputs, including createdDayUtc, before
signPublicEvidenceBundle invokes getOrCreatePublicPublisher. In
createPublicEvidenceRevocation, load the existing publisher identity without
creating one, verify it owns targetBundle, and only then perform signing;
rejected requests must not create publisher key files.
Source: Linters/SAST tools
| test("JCS rejects lone UTF-16 surrogate code units", () => { | ||
| expect(() => jcsStringify("\uDEAD")).toThrow(/unicode|surrogate/i); | ||
| expect(() => jcsStringify({ ["\uDEAD"]: true })).toThrow(/unicode|surrogate/i); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject lone UTF-16 surrogate code units in JCS input.
Line 180 fails because jcsStringify("\uDEAD") serializes an invalid Unicode scalar instead of rejecting it. This permits non-conformant canonical JSON and can produce different signing behavior across implementations.
Reject lone high and low surrogate code units before canonicalization.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 180-180: error: expect(received).toThrow(expected)
Expected pattern: /unicode|surrogate/i
Received function did not throw
Received value: ""\udead""
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:180:42)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 179 - 182,
Update jcsStringify to validate all input strings for lone high or low UTF-16
surrogate code units before canonicalization, including both standalone values
and object keys. Reject invalid surrogates with an error matching the existing
unicode/surrogate expectation while preserving valid surrogate pairs and normal
serialization.
Source: Linters/SAST tools
| const missing = rebuildRecord(base, { assertions: [] }); | ||
| const duplicate = rebuildRecord(base, { assertions: [ | ||
| { id: "method", required: true, passed: true }, | ||
| { id: "method", required: true, passed: false }, | ||
| ] }); | ||
|
|
||
| expect(() => signPublicEvidenceBundle({ | ||
| records: [missing], artifacts: [], createdDayUtc: "2026-08-12", configDir: missingHome, | ||
| })).toThrow(/assertion.*authority|missing.*assertion/i); | ||
| expect(existsSync(labPublicPublisherKeyPath(missingHome))).toBe(false); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject records that omit reviewed assertions before signing.
Line 196 fails because signPublicEvidenceBundle signs a VERIFIED record with no assertions. This lets a publisher create evidence that lacks the assertion coverage required by the reviewed authority.
Make validatePublicEvidenceAuthorities require the complete reviewed assertion set before publisher creation and signing.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 196-196: error: expect(received).toThrow(expected)
Expected pattern: /assertion.*authority|missing.*assertion/i
Received function did not throw
Received value: {
schemaVersion: "public_evidence_bundle_v1",
exportPolicyVersion: "public_export_policy_v1",
createdDayUtc: "2026-08-12",
publisher: {
algorithm: "ed25519",
keyId: "29aa7ecb1bca4e2919d1b5148e8ca346f91b6ca4ddb38ab9ceb78a8738119eed",
publicKey: "MCowBQYDK2VwAyEAY3LDnkjWiWQ78gqwY/8V/WZ8We0B8Lo6PuT3/0WdnoA=",
},
records: [
{
recordId: "28c70661dc58fcf85830f0ae63a33c4d1df02eb5ed37fccd58185b413df8ee85",
subjectId: "982a06b98a218df5ed68ae88f5f203e1911a3e875343c6ed8d5d0b74ff4c2b25",
evidenceLayer: "protocol_conformance",
suiteId: "responses-core",
suiteVersion: "1.0.0",
scenarioId: "responses-core.protocol.request-shape",
scenarioVersion: "1.0.0",
verdict: "VERIFIED",
observedDayUtc: "2026-08-12",
subject: {
subjectKind: "protocol",
compatibilityVersion: "2.13.0",
adapterFamily: "openai-chat",
inboundProtocol: "openai-responses",
upstreamProtocol: "openai-chat",
surface: "responses-http",
},
assertions: [],
}
],
artifacts: [],
bundleId: "72b237b04f95ed6f52b261695771b6b86cece4e1dc5d85337be04cacd29ac793",
bundleDigest: "c364057dc3f58d8d750a371be7c5262389a45e87eef24d1f5d12e6c8e22725c4",
signature: {
algorithm: "ed25519",
signedDigest: "c364057dc3f58d8d750a371be7c5262389a45e87eef24d1f5d12e6c8e22725c4",
signature: "aAuuCzYbxuNjNvp7oOe8H1lXaPgXnyiGsho+rDnPSybQVLQW+xKb5A2+hFbBHLItSU+uXD9XB+Ohtezx8RS4DA==",
},
}
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:196:9)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 188 - 197,
Update validatePublicEvidenceAuthorities to reject VERIFIED records whose
assertions omit any required reviewed assertion before publisher creation or
signing. Ensure signPublicEvidenceBundle propagates this validation failure,
while preserving rejection of duplicate assertions and existing valid-record
behavior.
Source: Linters/SAST tools
| test("community import enforces the cache file quota before creating another object", () => { | ||
| const publisher = configDir("ocx-cl10-cache-publisher-"); | ||
| const consumer = configDir("ocx-cl10-cache-consumer-"); | ||
| const community = labCommunityDir(consumer); | ||
| mkdirSync(community, { recursive: true, mode: 0o700 }); | ||
| for (let index = 0; index < 512; index += 1) { | ||
| writeFileSync(join(community, `occupied-${String(index).padStart(3, "0")}`), "x", { mode: 0o600 }); | ||
| } | ||
| const bundle = signPublicEvidenceBundle({ | ||
| records: [fixedRecord()], artifacts: [], createdDayUtc: "2026-08-12", configDir: publisher, | ||
| }); | ||
| expect(() => importCommunityEvidenceBundle(bundle, consumer)).toThrow(/cache.*bound|cache.*limit|capacity/i); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Count all community-cache entries when enforcing the file quota.
Line 216 fails because the importer creates a new bundle after the community directory already contains 512 entries. The quota does not bound the directory that receives imported evidence.
Check the complete cache entry count immediately before persistence. Reject the import when the configured capacity is reached.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 216-216: error: expect(received).toThrow(expected)
Expected pattern: /cache.*bound|cache.*limit|capacity/i
Received function did not throw
Received value: {
path: "/tmp/ocx-cl10-cache-consumer-NsPa2t/lab/community/bundle-6f3ef2523137e7b1b69fd9fd398cc5104712f919e2c2bd8900eadcb31599bc02-ab99824f2d796cce31e234fe25cce961429189d4f409617584dfb4c9b91bba20.json",
created: true,
status: "cryptographically_valid",
bundleId: "ab99824f2d796cce31e234fe25cce961429189d4f409617584dfb4c9b91bba20",
publisherKeyId: "6f3ef2523137e7b1b69fd9fd398cc5104712f919e2c2bd8900eadcb31599bc02",
}
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:216:67)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 205 - 217,
Update importCommunityEvidenceBundle to count all entries in the community cache
directory immediately before persisting the imported bundle, including existing
files and directories. Reject the import when the configured cache capacity is
already reached, before creating any new cache object.
Source: Linters/SAST tools
| test("duplicate-key diagnostics are bounded and do not reflect attacker-controlled key contents", () => { | ||
| const key = `SECRET-${"x".repeat(64 * 1024)}`; | ||
| const raw = Buffer.from(`{${JSON.stringify(key)}:1,${JSON.stringify(key)}:2}`, "utf8"); | ||
| try { | ||
| parseStrictPublicJson(raw); | ||
| throw new Error("expected duplicate-key rejection"); | ||
| } catch (error) { | ||
| expect(error).toBeInstanceOf(Error); | ||
| const message = (error as Error).message; | ||
| expect(message.length).toBeLessThan(256); | ||
| expect(message).not.toContain("SECRET-"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Do not include duplicate key text in parser diagnostics.
Line 228 receives a 65,570-character error message. Line 229 also shows that the parser reflects the attacker-controlled key. This can amplify logs and expose untrusted input through error channels.
Return a bounded generic duplicate-key diagnostic, optionally with a byte offset. Do not include the key value.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 228-228: error: expect(received).toBeLessThan(expected)
Expected: < 256
Received: 65570
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:228:30)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 219 - 230,
Update the duplicate-key error path used by parseStrictPublicJson to return a
bounded generic diagnostic, optionally including only a byte offset. Remove the
duplicate key value from the error message so attacker-controlled key contents
are neither reflected nor able to amplify logs, while preserving duplicate-key
rejection.
Source: Linters/SAST tools
| test("privacy scanner rejects unbracketed IPv6 literals", () => { | ||
| const base = fixedRecord(); | ||
| const subject = { ...base.subject, surface: "2001:db8::1" }; | ||
| const subjectId = publicEvidenceId("subject", subject); | ||
| const record = rebuildRecord(base, { subject, subjectId }); | ||
| expect(() => validatePublicEvidenceRecordPrivacy(record)).toThrow(/IP address|privacy/i); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Detect unbracketed IPv6 literals in public evidence fields.
Line 238 fails because validatePublicEvidenceRecordPrivacy accepts 2001:db8::1. A public evidence record can therefore export an IP address through subject.surface.
Extend the privacy scanner to reject valid IPv6 literals in both bracketed and unbracketed forms.
🧰 Tools
🪛 GitHub Check: test 4/4
[failure] 238-238: error: expect(received).toThrow(expected)
Expected pattern: /IP address|privacy/i
Received function did not throw
Received value: undefined
at <anonymous> (/home/runner/work/opencodex/opencodex/tests/lab-public-deep-review-regressions.test.ts:238:63)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/lab-public-deep-review-regressions.test.ts` around lines 233 - 239,
Update validatePublicEvidenceRecordPrivacy to detect and reject valid IPv6
literals in public evidence fields, including unbracketed values such as
subject.surface and bracketed forms. Preserve the existing IPv4 and
privacy-validation behavior while ensuring rejected values produce the
established privacy/IP validation error.
Source: Linters/SAST tools
2 participants
Summary
Defines the CL-10 contract for public Compatibility Lab evidence export, publishing, and community trust.
This is the final planned Compatibility Lab phase after merged CL-09.
Base
dev:4fed8d3fe431ad23be83f3aff2af18ef8b8ecd71feat/cl-10-public-evidence-contractScope
Contract/design only. No runtime export, upload, remote fetch, signing-key creation, community cache, API, CLI, or UI implementation is included or authorized by this PR alone.
The contract freezes:
PublicEvidenceBundleV1export schemapublic_exportartifact policy plus second-pass privacy scanningcommunity_untrusted_v1trust semantics for imported evidenceChosen architecture
Delivery sequence after contract acceptance
Files
devlog/_plan/260807_compatibility_lab/010_cl10_public_evidence_export.mddocs/superpowers/specs/2026-08-12-cl10-public-evidence-design.mdReview focus
Please review this primarily as a privacy/trust boundary, not as a product-surface proposal.
Key questions:
Validation
Contract PR minimum:
devDo not merge based only on the contract draft. Runtime CL-10 implementation starts only after independent contract acceptance.
Summary by CodeRabbit
New Features
Security & Privacy