Repository navigation
feat: add seeded coupling attestation - #1
Conversation
Add closed-service attestation handling, per-install watermark seed derivation, seeded native image and mesh watermark entrypoints, and runtime proof sealing. Refresh the Windows native artifacts and cover attestation, seed derivation, seeded round trips, and native channel interop. Verified with bun run check and bun test.
|
@coderabbitai review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds attestation handlers, coupling seed derivation and seed-aware scan/materialization wiring, and native watermark v2 image and mesh codecs. It also updates service, integration, and native runtime tests plus checksum files. ChangesService-side attestation and seed plumbing
Native watermark runtime
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0522674fb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
src/server.ts-831-835 (1)
831-835: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn 400 for malformed derive-seeds JSON.
Unlike the attestation routes, this direct
request.json()call is not caught, so invalid JSON can escape as an internal error instead of a controlled bad request response.🤖 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/server.ts` around lines 831 - 835, The direct request.json() parse in the derive-seeds handler can throw and escape as an internal error instead of a bad request. Wrap the JSON parsing in the same error handling pattern used by the attestation routes in src/server.ts, and return a 400 response for malformed JSON while keeping the existing derive-seeds flow intact. Use the derive-seeds request handler and the request.json() call as the locating symbols.src/attestation.native.test.ts-17-24 (1)
17-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid silently skipping all native interop coverage.
The DLL path points at
../../CreatorAssistant/.../CouplingRuntimeCom.dll; if CI only has the refreshed runtime artifacts under the repo’s native output, this suite becomes a no-op. Resolve the built artifact path used by this PR or require an explicit env override before skipping.Also applies to: 55-55
🤖 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/attestation.native.test.ts` around lines 17 - 24, The native attestation test is using a hardcoded DLL_PATH that points to an outdated external artifact location, which can cause the entire suite to skip and lose coverage. Update the path resolution in src/attestation.native.test.ts to use the built artifact location produced by this PR, or require an explicit environment override before deciding the DLL is missing. Keep the skip logic tied to DLL_PATH/dllExists so the test only bypasses when the configured native binary is genuinely unavailable.src/attestation.test.ts-151-198 (1)
151-198: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRestore mutated environment variables after each test.
This file sets attestation/control-plane env vars but only restores
fetch. Those env values can leak into later tests and change behavior depending on execution order.🤖 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/attestation.test.ts` around lines 151 - 198, The attestation test setup mutates several process.env values in beforeEach but only restores globalThis.fetch in afterEach, so those settings can leak across tests. Save the original values for ATTESTATION_CHANNEL_PRIVATE_KEY_PKCS8, ATTESTATION_FINGERPRINT_SALT, YUCP_TPM_ROOTS_JSON, CONVEX_API_SECRET, and CONVEX_SITE_URL in the test setup and restore them in afterEach alongside fetch, using beforeEach/afterEach in src/attestation.test.ts to keep the test environment isolated.src/attestation.native.test.ts-174-231 (1)
174-231: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse
finallyfor DLL and temp-directory cleanup.If any assertion fails before Line 225 or Line 231, the DLL handle/temp directory can leak; on Windows this can also keep the native artifact locked.
🤖 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/attestation.native.test.ts` around lines 174 - 231, The test cleanup for the native DLL and temp directory is not protected against assertion failures, so resources can leak or stay locked on Windows. Wrap the body of this test around the existing seal/handleCouplingProof assertions in a try/finally, and move lib.close() and rmSync(dir, { recursive: true, force: true }) into the finally block. Keep the existing test logic and use the current lib, dir, and seal helpers unchanged.src/attestation.native.test.ts-42-53 (1)
42-53: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRestore test environment mutations.
The native tests set
ATTESTATION_*,YUCP_*, andCONVEX_*env vars but do not restore their previous values, which can affect later tests in the same Bun run.🤖 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/attestation.native.test.ts` around lines 42 - 53, The native test setup is mutating process environment state without restoring it, which can leak ATTESTATION_*, YUCP_*, and CONVEX_* values into later Bun tests. Update the test lifecycle around the existing beforeEach/afterEach in attestation.native.test.ts to snapshot the prior env values before setting them and restore or delete them afterward, alongside the existing fetch restoration, so each test leaves the environment unchanged.src/ffi.ts-305-307 (1)
305-307: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize
seedHexbefore passing it to the decoder FFI.
encodeCouplingAsset()already runsnormalizeSeedHexForFfi(), butcallNativeDecoder()forwardsinput.seedHexas-is. That makes v2 decode sensitive to casing/whitespace differences and bypasses the new 64-hex validation entirely.Suggested fix
- const seedPointer = input.seedHex ? ptr(toNativeUtf8Buffer(input.seedHex)) : null; + const normalizedSeedHex = input.seedHex ? normalizeSeedHexForFfi(input.seedHex) : null; + const seedPointer = normalizedSeedHex ? ptr(toNativeUtf8Buffer(normalizedSeedHex)) : null;🤖 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/ffi.ts` around lines 305 - 307, `callNativeDecoder()` is forwarding `input.seedHex` directly, which bypasses the same normalization and validation used by `encodeCouplingAsset()`. Update the decoder path to run `normalizeSeedHexForFfi()` before building `seedPointer`, so v2 decode accepts only the normalized 64-hex seed and is not sensitive to casing or whitespace. Keep the fix localized in `callNativeDecoder()` and reuse the existing normalization helper rather than duplicating parsing logic.
🧹 Nitpick comments (1)
src/nativeHardening.test.ts (1)
41-49: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAlso assert the runtime-helper export map drops the legacy entrypoints.
Line 42 only checks source text, so a stale
.def/artifact export surface could still shipEntryPointwhile this test passes. Add the runtime-helper.defto the assertion set at minimum.Suggested test hardening
expect(runtimeSource).not.toContain('EntryPoint'); expect(runtimeSource).not.toContain('yw_run_manifest'); expect(runtimeSource).not.toContain('RESULT|'); expect(runtimeSource).not.toContain('FILE|'); + + const runtimeHelperExports = readNativeFile('yucp_coupling', 'yucp_coupling.runtime-helper.def'); + expect(runtimeHelperExports).not.toMatch(/\bEntryPointW?\b/);🤖 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/nativeHardening.test.ts` around lines 41 - 49, The native hardening test only checks coupling_runtime.c text, so a stale export surface could still expose legacy entrypoints; harden the test by also asserting the runtime-helper .def/export map does not include EntryPoint or other legacy rundll32 symbols. Update the existing nativeHardening.test.ts case alongside readNativeFile-based checks so it validates both the runtime source and the def/artifact export list, using the same runtime-helper symbols referenced in the current test.
🤖 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 `@src/attestation.ts`:
- Around line 421-429: The fingerprint vector in attestation.ts is currently
persisting client-supplied component weights via payload.components and
c.weight, which should not be trusted. Update the fingerprint-building logic
around the Promise.all mapping so weights are assigned server-side from a fixed
component policy or lookup based on c.component, and only the server-derived
weight is stored alongside the salted hash. Keep the existing saltedHash and
fingerprintVector flow, but remove any direct dependence on c.weight.
- Around line 216-258: The EK/AK verification in verifyEkChainAndAkBinding only
checks that ekCertDerB64 is present, so update this function to actually parse
and validate the EK certificate chain against the pinned TPM roots. Use the
existing verifyEkChainAndAkBinding flow to confirm the submitted EK cert is a
valid X.509 cert, build its chain to a trusted root from loadPinnedTpmRoots(),
and only then continue with the AK binding checks. Keep the existing
akCertifyB64 and akCertifySigB64 verification, but make tpmVerified depend on
both the EK chain validation and the AK signature check.
- Around line 598-604: The coupling proof asset handling in attestation.ts
currently accepts empty or arbitrary sha256 values and turns them into
contentSha256 entries, so add validation before building assets in the
Promise.all over payload.couplingAssets. Enforce a bounded number of assets and
reject any asset whose sha256 is not exactly 64 hex characters (allowing
uppercase or lowercase) before calling saltedHash or persisting the relay
payload. Use the existing couplingAssets mapping and the assets construction
block to locate the fix, and fail fast rather than normalizing invalid hashes to
an empty string.
- Around line 272-281: The relay() call to the control plane can hang
indefinitely, so add an AbortController-based timeout around the fetch in
relay() and ensure stalled requests are cancelled. Update relay() to use a
bounded timeout and map timeout/abort failures to a controlled 502/504-style
response or error path so challenge/submit/payment/proof flows fail fast. Use
the relay() helper and its fetch call as the main place to apply the fix.
- Around line 346-355: The decrypted attestation payload is only being type-cast
in `src/attestation.ts` and not actually validated, so malformed data can reach
later `.map()` calls and fail as a 500. Update the payload parsing flow around
the `SubmitPayload` handling to verify the parsed object has the expected shape,
especially that `components` and `osAnchors` are arrays, before assigning it or
using it. If validation fails, return the same 422-style bad request response
instead of continuing.
- Around line 442-447: The EK anchor is being derived from the AK value, so
rotate-resistant TPM identity is not actually stable. Update the ekHash
calculation in attestation logic to use the verified EK certificate/public key
rather than payload.akPublicSpkiB64, keeping the saltedHash call and
ANCHOR_HASH_PURPOSE but changing the input source to the EK data available in
this flow. Make sure the tpm_ek anchor continues to be set only when tpmVerified
is true, and reference the existing ekHash path so the durable TPM identity no
longer changes with AK rotation.
In `@src/couplingSeed.ts`:
- Line 54: The HKDF info construction in couplingSeed’s seed derivation uses a
pipe-delimited string that can collide if assetId or licenseSubject contains the
delimiter. Update the info encoding in couplingSeed to an injective format such
as a JSON array or length-prefixed fields so each (assetId, licenseSubject,
epoch) tuple maps to a unique HKDF info value.
In `@src/materialize.ts`:
- Around line 540-551: The protected materialization flow in materialize.ts is
failing open because the coupling job loop in the redeem/unlock path skips all
jobs when isCouplingSeedConfigured() is false or redeemed.licenseSubject is
blank, then still returns unmarked output. Change the logic around
redeemed.couplingJobs and seedAvailable so that any non-empty couplingJobs set
requires a valid coupling seed and licenseSubject; otherwise throw before
receiptGrant() completes. Keep the existing target validation in the loop, but
make the no-seed case an explicit failure instead of continuing.
In `@src/roundtrip.test.ts`:
- Around line 228-230: The seed E2E test has duplicated response parsing and
repeated type members, and it also reads the same response body more than once.
Clean up the affected blocks by keeping a single JSON parse per response and
reusing the parsed value instead of calling otherDerive.json() again; use the
existing derivePayload and related result variables to locate and consolidate
the repeated seed/results handling in the roundtrip test.
In `@src/server.ts`:
- Line 4: The HTTP router is missing the coupling-proof endpoint because
`handleCouplingProof` is not imported or registered alongside
`handleAttestationChallenge`, `handleAttestationSubmit`, and
`handlePaymentAnchor`. Update the router in `src/server.ts` to import
`handleCouplingProof` from `src/attestation.ts` and wire it into the same
routing setup used for the other attestation handlers so native runtime proof
sealing can be submitted through the service API.
- Around line 857-862: Seed derivation in the asset path loop uses raw.trim()
directly, but the scan flow normalizes paths through
normalizeRelativeAssetPath() and validates them with isSafeRelativeAssetPath().
Update the seed generation path in server.ts to apply the same
normalization/validation before calling deriveCouplingSeedHex, and use the
normalized assetPath consistently when pushing into seeds so embed/decode stay
aligned.
In `@yucp_coupling/guard.c`:
- Line 1289: The generated guard.c code contains duplicate typedef and enum
declarations that are being emitted in the same scope, causing redeclaration
errors. Update the code generation around wm_domain and the related symbols
wm_imgfmt, wm_vref, wm_vlist, and wm_pair so each type is declared only once,
and ensure the enum enumerators are not emitted again in later blocks such as
the ones corresponding to the repeated spots mentioned in the review.
- Around line 2749-2752: Guard the allocation-size multiplications in
wm_perm_take and the other affected allocation sites so count, nc, and N cannot
overflow when multiplied by sizeof(...) before malloc/realloc. Add SIZE_MAX /
sizeof(...) checks immediately before each allocation expression, and return the
existing failure path if the product would overflow, so the buffers are never
under-allocated before later indexed use.
- Around line 2890-2945: The parsed per-job seed in the watermark/image path is
left on the stack across all return paths, which leaks the placement secret;
update the entrypoint around the seed parsing and image embed/extract flow to
clear it before every exit. Add centralized cleanup or explicitly call
sodium_memzero on seed with sizeof(seed) before each return in the relevant
functions (including the other listed entrypoints that follow the same pattern),
alongside the existing frees and image buffer cleanup.
- Around line 4170-4172: The mesh bin ordering in wm_pair_cmp is
nondeterministic when two entries have equal k values, which can cause different
qsort results across runtimes. Update the comparator used by the vertex/bin
reorder path (including the related logic around the apply/reparse flow) to add
a deterministic tie-breaker for equal radii, using a stable secondary key from
the wm_pair data so the intended vertex-reorder robustness is preserved while
producing the same order everywhere.
- Around line 1345-1353: The wm_unframe() parser in guard.c only checks version
and CRC, so it accepts frames with an incorrect LEN value; add explicit
validation of the framed length byte during unframe to match what wm_frame()
emits (LEN=64). Locate the length parsing logic in wm_unframe() and reject any
input whose length field does not equal the expected constant before continuing
with CRC verification.
- Around line 2904-2907: Add a dimension preflight in the image decoding paths:
use stbi_info_from_memory() before each stbi_load_from_memory() call in guard.c
so width/height are validated before decoding, not just after. Update both
decode sites in the functions around the first and second image loads to reject
oversized dimensions up front, since WM_IMG_MAX_BYTES only constrains compressed
input and does not limit decoded pixel count.
---
Minor comments:
In `@src/attestation.native.test.ts`:
- Around line 17-24: The native attestation test is using a hardcoded DLL_PATH
that points to an outdated external artifact location, which can cause the
entire suite to skip and lose coverage. Update the path resolution in
src/attestation.native.test.ts to use the built artifact location produced by
this PR, or require an explicit environment override before deciding the DLL is
missing. Keep the skip logic tied to DLL_PATH/dllExists so the test only
bypasses when the configured native binary is genuinely unavailable.
- Around line 174-231: The test cleanup for the native DLL and temp directory is
not protected against assertion failures, so resources can leak or stay locked
on Windows. Wrap the body of this test around the existing
seal/handleCouplingProof assertions in a try/finally, and move lib.close() and
rmSync(dir, { recursive: true, force: true }) into the finally block. Keep the
existing test logic and use the current lib, dir, and seal helpers unchanged.
- Around line 42-53: The native test setup is mutating process environment state
without restoring it, which can leak ATTESTATION_*, YUCP_*, and CONVEX_* values
into later Bun tests. Update the test lifecycle around the existing
beforeEach/afterEach in attestation.native.test.ts to snapshot the prior env
values before setting them and restore or delete them afterward, alongside the
existing fetch restoration, so each test leaves the environment unchanged.
In `@src/attestation.test.ts`:
- Around line 151-198: The attestation test setup mutates several process.env
values in beforeEach but only restores globalThis.fetch in afterEach, so those
settings can leak across tests. Save the original values for
ATTESTATION_CHANNEL_PRIVATE_KEY_PKCS8, ATTESTATION_FINGERPRINT_SALT,
YUCP_TPM_ROOTS_JSON, CONVEX_API_SECRET, and CONVEX_SITE_URL in the test setup
and restore them in afterEach alongside fetch, using beforeEach/afterEach in
src/attestation.test.ts to keep the test environment isolated.
In `@src/ffi.ts`:
- Around line 305-307: `callNativeDecoder()` is forwarding `input.seedHex`
directly, which bypasses the same normalization and validation used by
`encodeCouplingAsset()`. Update the decoder path to run
`normalizeSeedHexForFfi()` before building `seedPointer`, so v2 decode accepts
only the normalized 64-hex seed and is not sensitive to casing or whitespace.
Keep the fix localized in `callNativeDecoder()` and reuse the existing
normalization helper rather than duplicating parsing logic.
In `@src/server.ts`:
- Around line 831-835: The direct request.json() parse in the derive-seeds
handler can throw and escape as an internal error instead of a bad request. Wrap
the JSON parsing in the same error handling pattern used by the attestation
routes in src/server.ts, and return a 400 response for malformed JSON while
keeping the existing derive-seeds flow intact. Use the derive-seeds request
handler and the request.json() call as the locating symbols.
---
Nitpick comments:
In `@src/nativeHardening.test.ts`:
- Around line 41-49: The native hardening test only checks coupling_runtime.c
text, so a stale export surface could still expose legacy entrypoints; harden
the test by also asserting the runtime-helper .def/export map does not include
EntryPoint or other legacy rundll32 symbols. Update the existing
nativeHardening.test.ts case alongside readNativeFile-based checks so it
validates both the runtime source and the def/artifact export list, using the
same runtime-helper symbols referenced in the current test.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 185303d4-494a-4ed4-8005-3c585fd2d891
⛔ Files ignored due to path filters (4)
yucp_coupling/out/win-x64/Release/coupling_runtime.objis excluded by!**/*.objyucp_coupling/out/win-x64/Release/runtime-helper/coupling_runtime.objis excluded by!**/*.objyucp_coupling/out/win-x64/Release/runtime-helper/yucp_coupling.dllis excluded by!**/*.dllyucp_coupling/out/win-x64/Release/yucp_coupling.dllis excluded by!**/*.dll
📒 Files selected for processing (25)
coupling_runtime.expcoupling_runtime.libsrc/attestation.native.test.tssrc/attestation.test.tssrc/attestation.tssrc/config.tssrc/couplingSeed.test.tssrc/couplingSeed.tssrc/ffi.tssrc/materialize.tssrc/nativeHardening.test.tssrc/roundtrip.test.tssrc/server.tsyucp_coupling/coupling_runtime.cyucp_coupling/coupling_runtime.expyucp_coupling/coupling_runtime.libyucp_coupling/guard.cyucp_coupling/out/win-x64/Release/runtime-helper/yucp_coupling.compile.pdbyucp_coupling/out/win-x64/Release/runtime-helper/yucp_coupling.pdbyucp_coupling/out/win-x64/Release/runtime-helper/yucp_coupling.sha256yucp_coupling/out/win-x64/Release/yucp_coupling.compile.pdbyucp_coupling/out/win-x64/Release/yucp_coupling.pdbyucp_coupling/out/win-x64/Release/yucp_coupling.sha256yucp_coupling/yucp_coupling.defyucp_coupling/yucp_coupling.runtime-helper.def
Address review feedback by routing sealed coupling proof submissions, validating decrypted attestation arrays before hashing, and avoiding the legacy FBX token-length default for seeded v2 scans. Adds regressions for malformed sealed attestation payloads, seeded FBX scans without explicit expectedTokenLength, and the public coupling-proof route. Verified with bun run check and bun test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e74d9e24dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bound relay calls with AbortController timeouts, keep attestation component weighting server-side, and anchor TPM identity on EK certificate material so AK rotation does not split a machine identity. Validate coupling proof assets before relay, normalize seed derivation paths at the HTTP boundary, and use structured HKDF info to avoid delimiter collisions. Fail closed when protected materialization includes coupling jobs without seed material or a buyer subject, normalize decoder seed input before FFI calls, and add native framing/allocation/tie-break hardening guards.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e89abae651
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bind coupling proof TPM signatures to the canonical proof body so nonce-only signatures cannot verify swapped runtime or asset fields. Validate coupling seed availability and signed job targets before decrypted assets are copied into the project tree. Preflight v2 image dimensions before decoding, wipe parsed native seed material on seeded entrypoint exits, and refresh the Linux runtime artifacts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57bc4b359f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nativeHardening.test.ts (1)
94-109: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the committed SHA-256 files against the checked-in artifacts.
This test only proves the Linux
.soexists and contains the new symbol names. It would still miss drift between the committed binaries and the updated.sha256files inout/linux-x64/Release, including the runtime-helper checksum changed in this PR. Adding a digest comparison here would catch stale artifact/checksum pairs before release.🤖 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/nativeHardening.test.ts` around lines 94 - 109, The native packaged artifacts test currently only verifies the Linux .so and symbol presence, but it does not check that the checked-in .sha256 files match the committed binaries. Update the nativeHardening.test.ts coverage in the native packaged artifacts / Linux server artifacts assertion to also read the SHA-256 sidecar files in out/linux-x64/Release and compare their recorded digests against the actual artifact bytes, including the runtime-helper checksum, so stale artifact/checksum mismatches are caught.
🤖 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.
Nitpick comments:
In `@src/nativeHardening.test.ts`:
- Around line 94-109: The native packaged artifacts test currently only verifies
the Linux .so and symbol presence, but it does not check that the checked-in
.sha256 files match the committed binaries. Update the nativeHardening.test.ts
coverage in the native packaged artifacts / Linux server artifacts assertion to
also read the SHA-256 sidecar files in out/linux-x64/Release and compare their
recorded digests against the actual artifact bytes, including the runtime-helper
checksum, so stale artifact/checksum mismatches are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 758400ab-da82-434e-bc37-93c1b40d7c32
⛔ Files ignored due to path filters (2)
yucp_coupling/out/linux-x64/Release/runtime-helper/yucp_coupling.sois excluded by!**/*.soyucp_coupling/out/linux-x64/Release/yucp_coupling.sois excluded by!**/*.so
📒 Files selected for processing (14)
src/attestation.test.tssrc/attestation.tssrc/couplingSeed.test.tssrc/couplingSeed.tssrc/ffi.test.tssrc/ffi.tssrc/materialize.test.tssrc/materialize.tssrc/nativeHardening.test.tssrc/roundtrip.test.tssrc/server.tsyucp_coupling/guard.cyucp_coupling/out/linux-x64/Release/runtime-helper/yucp_coupling.sha256yucp_coupling/out/linux-x64/Release/yucp_coupling.sha256
✅ Files skipped from review due to trivial changes (1)
- yucp_coupling/out/linux-x64/Release/yucp_coupling.sha256
🚧 Files skipped from review as they are similar to previous changes (5)
- src/couplingSeed.test.ts
- src/couplingSeed.ts
- src/ffi.ts
- src/attestation.ts
- src/server.ts
Relay a SHA-256 machine fingerprint join value from sealed attestations so the open server can require current-machine attestation without receiving raw hardware data. Parse submitted EK certificates as X.509 and verify them against pinned TPM roots before accepting the existing AK certify signature. Use real root and EK certificate fixtures in the attestation tests so TPM verification exercises the certificate path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f17a4bda06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Reject coupling proof persistence unless the TPM-backed signature verifies the canonical proof body, so no-TPM or nonce-only submissions cannot write forensic asset records. Pre-derive all coupling seeds before decrypting and copying protected assets so malformed seed configuration cannot publish unwatermarked files before failing. Update regressions for no-TPM proof rejection, nonce-only proof rejection, authenticated proof success, and pre-copy seed validation.
Normalize coupling job tokens and derive seeds before extraction can publish assets, then apply watermarking inside the temporary extraction tree before copying files into the project. Route legacy-length tokens through the legacy native encoders while keeping 16-hex tokens on the seeded v2 encoders, preserving older protected packages without weakening new seeded jobs. Add regressions for malformed token preflight and legacy-length materialization.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/materialize.test.ts`:
- Around line 321-348: The test leaves process-wide state behind by mutating
both globalThis.fetch and the shared config singleton in the
materializeProtectedPayload setup. In this test, add cleanup in the finally
block to restore the original fetch implementation and reset or restore config
(especially wmMasterKeyHex/workRoot) after the assertions, so later tests are
not affected by the stubbed fetch or invalid COUPLING_WM_MASTER_KEY.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: faf672b0-e89d-4466-94ee-c675eac1dadb
📒 Files selected for processing (5)
src/attestation.native.test.tssrc/attestation.test.tssrc/attestation.tssrc/materialize.test.tssrc/materialize.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/materialize.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c782388f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Export the seeded PNG and FBX encoder wrappers from the Linux runtime-helper artifact so the advertised helper exports resolve at runtime. Rebuild the Linux helper artifact and harden native artifact tests to inspect ELF dynamic exports instead of raw string presence. Also restore materialization test globals in finally to prevent order-dependent test leakage.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9810ce15f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sigOk = await verifyAkSignatureOverNonce( | ||
| payload.akPublicSpkiB64, | ||
| payload.challengeSigB64, | ||
| payload.nonce, | ||
| ); |
There was a problem hiding this comment.
In the attestation-submit path, a patched importer running on a genuine TPM only needs the AK to sign payload.nonce; the sealed channel can be produced by any client with the service public key, so components, osAnchors, and the claimed runtime self-hash remain unauthenticated JSON while tpmVerified is recorded as true. That lets the caller store arbitrary fingerprints under a real TPM anchor; verify a canonical attestation message that includes the nonce plus the collected fields/self hash instead of only the nonce.
Useful? React with 👍 / 👎.
| for (const job of couplingJobSeeds) { | ||
| encodeCouplingAsset({ | ||
| filePath: resolveProjectPath(extractRoot, job.assetPath), | ||
| assetType: inferEncodableAssetType(job.assetPath), |
There was a problem hiding this comment.
For protected materialization jobs targeting JPG/TGA/BMP assets, the new v2 native image encoder can handle those containers, but this path still infers the asset type only from .png/.fbx; inferEncodableAssetType() therefore throws before encoding and prevents the buyer from materializing packages with those raster coupling jobs. Treat the supported raster extensions as the image encoder asset type here, or include an explicit asset type in the job.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
bun run checkbun testSummary by CodeRabbit