fix(injection-points): KDF chain markdown rendering + work-factor-not-entropy wording (supersedes #5608)#5621
Merged
AceHack merged 2 commits intoMay 27, 2026
Conversation
…rate markdown rendering + work-factor-not-entropy wording (addresses Copilot findings on PR #5608; supersedes #5608) Supersedes PR #5608. Two valid Copilot findings on that PR: Finding 1 (markdown rendering): My earlier fix put backslash-escaped pipes (\|\| and "\|") inside inline code spans in a table cell. Markdown code spans render backslashes literally, so readers saw "\|\|" / "\"\|\"" instead of intended "||" / "|". Finding 2 (entropy wording): "stretches low-entropy passphrase into high-entropy intermediate" is misleading — scrypt does NOT increase the underlying entropy of a weak passphrase (in information-theoretic terms, a weak passphrase remains weak). What scrypt provides is a tunable work-factor cost per guess, making brute-force memory- prohibitively expensive on GPU/ASIC. Both addressed by restructuring: - Table cell at line 116 simplified to: "AES-256-GCM; key derived via 2-layer scrypt → HKDF chain (full mechanism + parameters below)" — no pipe-in-inline-code issue - New "KDF chain detail (mechanism + parameters)" sub-section below the table with full mechanism in code blocks (markdown code blocks don't have the pipe-escaping issue inline code in table cells has) - Wording corrected: scrypt provides "tunable work-factor cost per guess" + "makes brute-force memory-prohibitively expensive on GPU/ASIC" + "scrypt is the layer that makes the IKM cryptographically suitable for HKDF input" (per OWASP guidance; per the 2026-05-27 security-review HIGH finding rationale documented in the zeta-creds-crypto.ts source) LAYER 1 — scrypt: stretched = scrypt(passphrase, salt, length=32, N=2^17, r=8, p=1, maxmem=256MB) LAYER 2 — HKDF-SHA256: ikm = concat(usbUuid_utf8, "|", stretched) key = HKDF-SHA256(ikm, salt, info="zeta-b0852-cred-persistence-v1", length=32) Operator-named threat preserved verbatim: "we can put a key on the usb too if wnated tied to the uuid so it can't be copied to uuid" — the HKDF UUID-binding defense. Why supersedes (not fix-fwd): PR #5608 used force-push-restricted branch path; new-branch path is policy-respected alternative. Pattern matches PR #5620 superseding PR #5606. Copilot thread IDs to resolve when closing #5608: - PRRT_kwDOSF9kNM6FNUvq (line 116, backslash escaping) - PRRT_kwDOSF9kNM6FNUwj (line 116, entropy wording) Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
4 tasks
There was a problem hiding this comment.
Pull request overview
Updates the credential-blob “Encryption” entry in full-ai-cluster/INJECTION-POINTS.md to avoid Markdown rendering pitfalls in table inline-code and to describe the intended scrypt → HKDF key-derivation chain using a dedicated subsection.
Changes:
- Replaces the table-cell inline KDF expression with a pointer to a new “KDF chain detail” subsection.
- Adds a clearer two-layer derivation description (scrypt work-factor + HKDF USB-UUID binding) with pseudocode blocks.
- Adjusts wording to avoid implying that scrypt “increases entropy” of weak passphrases.
…ce brittle line-range reference with symbol+constants reference; cite OWASP cheat sheet for scrypt parameters + soften operational-cost claim Copilot caught 2 P1 findings on PR #5621 (the supersedes-#5608 fix). Both valid. Finding 1 (P1, line 124, brittle line reference): Was: `tools/installer/zeta-creds-crypto.ts:80-125` (line numbers drift; doesn't cover SCRYPT_N + SCRYPT_MAXMEM constants declared higher in the file) Fixed: `tools/installer/zeta-creds-crypto.ts` (the `deriveKey` function + the `SCRYPT_*` + `KEY_LEN` + `SALT_LEN` + `HKDF_INFO` constants declared near the top of the file) — symbol-based + constants-named; survives line-number drift. Finding 2 (P1, line 134, uncited operational + standard claims): Was: "OWASP 2026 recommended parameters: N=2^17, r=8, p=1." + "~1-2 seconds of CPU per derivation" (no citation; generalized across-machines) Fixed: - Concrete citation: [OWASP Password Storage Cheat Sheet] (https://cheatsheetseries.owasp.org/cheatsheets/ Password_Storage_Cheat_Sheet.html#scrypt) at parameter- selection date 2026-05-27 - Bump procedure named: visit cheat sheet at next security-review cadence; update both the cheat-sheet-citation date here AND the SCRYPT_N/SCRYPT_R/SCRYPT_P constants in zeta-creds-crypto.ts - Operational cost claim softened: "per the source-code comment's empirical timing context, on the maintainer's modern CPU at parameter-selection time, ~1-2 seconds of CPU per derivation" + "per-machine operational cost will vary with CPU + memory bandwidth" — substrate-honest; no across-machines generalization. Per .claude/rules/blocked-green-ci-investigate-threads.md verify- before-fix: both findings inspected via direct line-level reading; both confirmed real + fixed. Copilot thread IDs to resolve after merge: - PRRT_kwDOSF9kNM6FNhdV (line 124, brittle line reference) - PRRT_kwDOSF9kNM6FNhd3 (line 134, uncited OWASP + operational cost) Pre-existing MD060 IDE warnings (compact table column-style without spaces around pipes) NOT addressed in this commit — same warnings on all prior-merged commits to this file; not CI-blocking. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes #5608. Two valid Copilot findings on that PR addressed by restructuring the KDF documentation away from inline-code-in-table-cell into a sub-section with proper code blocks below the table.
Findings + fixes
Finding 1 (line 116) — markdown rendering
Was: `HKDF(USB-UUID \|\| operator-passphrase, salt, info)` inside a table cell with backslash-escaped pipes that render literally in markdown code spans (readers saw "\|\|" instead of "||").
Fixed: Table cell simplified to point at a sub-section below. KDF mechanism documented in code blocks (no pipe-escaping issue inline code in table cells has).
Finding 2 (line 116) — entropy wording misleading
Was: "stretches low-entropy passphrase into high-entropy intermediate" — implies scrypt increases entropy of weak passphrases.
Fixed: Corrected to substrate-honest wording: scrypt does NOT increase entropy of weak passphrases (information-theoretically); it provides tunable work-factor cost per guess, making brute-force memory-prohibitively expensive on GPU/ASIC. Per OWASP guidance + the 2026-05-27 security-review HIGH finding rationale documented in `zeta-creds-crypto.ts`.
Why new-branch path (not force-push)
Same as PR #5620 superseding PR #5606: force-push restricted by autonomous-loop discipline; new-branch path is policy-respected alternative.
Copilot threads on PR #5608 to resolve when closing
Test plan
tools/installer/zeta-creds-crypto.ts:80-125🤖 Generated with Claude Code