Add zip-free decrypt + re-wrap primitives for decomposed results#26
Open
chrisbendel wants to merge 12 commits into
Open
Add zip-free decrypt + re-wrap primitives for decomposed results#26chrisbendel wants to merge 12 commits into
chrisbendel wants to merge 12 commits into
Conversation
Additive on top of the prod manifest format (no byte changes to writer
output or the embedded manifest). Lets a researcher who was not an
original recipient decrypt the same ciphertext via a re-wrapped AES key.
- crypto.ts: wrapAesKey / unwrapAesKey / decryptFileBody / decryptFile
(decryptFile returns the raw AES key so the reviewer's browser can
re-wrap for researchers without a second decrypt).
- reader.ts: optional {filePath -> crypt} override map; extractFilesWithKeys()
exposes each file's raw AES key. Existing (blob, priv, fp) usage unchanged.
- writer.ts: delegates key wrapping to crypto.wrapAesKey (identical output).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d6e94c5 to
0e892f0
Compare
# Conflicts: # pnpm-lock.yaml # pnpm-workspace.yaml
Add pnpm overrides resolving three HIGH advisories in transitive dev deps: - esbuild >=0.28.1 (GHSA-gv7w-rqvm-qjhr) - vite 8.0.16 (CVE-2026-53571) - ws >=8.21.0 (CVE-2026-48779) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
decryptFile was uncovered (61% file coverage); round-trip test brings crypto.ts to ~100%, clearing the SonarQube quality gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AES-CBC is unauthenticated: ciphertext is malleable and exposed to padding-oracle attacks (SonarQube S5542). Move file bodies to AES-GCM, which authenticates ciphertext and IV on decrypt. - writer: generate AES-GCM key, 96-bit IV, stamp algo in manifest - crypto: unwrap/decrypt under shared AES_ALGORITHM constant - reader: reject manifests with an unexpected cipher - types: add self-describing `algo` field to ResultsFile - tests: GCM round-trip + tamper-detection (flipped byte must throw) No backward-compat path: no AES-CBC data exists yet (pre-release). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Diagnostic step to surface which gate condition is red. Revert after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the lone new-code violation failing the quality gate (typescript:S2933 — overrideKeys is never reassigned). Reverts the temporary debug steps used to surface the condition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit 0b6f71b. Existing production results are encrypted with AES-CBC; switching the cipher would make them undecryptable. Keep AES-CBC for backward compatibility. The readonly fix from a later commit is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the parallel overrideKeys lookup in ResultsReader with a merge-into-manifest model: caller-supplied keys are spliced into manifest.files[path].keys[fingerprint] on decode, so reads stay a single fingerprint lookup with no fingerprint-bypass and no key/IV mismatch risk. Ctor arg renamed overrideKeys -> additionalKeys (still positional, same Record<path, crypt> shape, so consumers are unaffected). Document why file bodies stay AES-CBC (backward-compat with existing production results; unauthenticated, S5542) on the writer and decrypt path. Add deterministic negative tests: override wrapped for the wrong keypair throws at RSA unwrap (never garbage plaintext), and a manifest file with no key for the recipient throws loudly rather than silently skipping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds lower-level crypto primitives to support zip-free decryption and AES key re-wrapping (so new recipients can decrypt existing encrypted results without re-encrypting file bodies), and refactors the existing zip-based ResultsReader/ResultsWriter to use those primitives. Also introduces pnpm overrides to bump a few dependency versions.
Changes:
- Add
job-results/crypto.tswithunwrapAesKey,wrapAesKey,decryptFileBody, anddecryptFile(returns{ contents, rawAesKey }). - Refactor
ResultsReader/ResultsWriteronto the new crypto primitives and add “override keys” support for recipients not present in the original manifest. - Add vitest coverage for wrap/unwrap, standalone decrypt, and the re-wrap override flow; update pnpm overrides/lockfile.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds dependency overrides for esbuild, vite, and ws. |
| pnpm-lock.yaml | Lockfile updates reflecting the new overrides and resolved versions. |
| job-results/writer.ts | Uses wrapAesKey primitive instead of an in-class RSA wrap helper; clarifies AES-CBC rationale. |
| job-results/reader.ts | Uses unwrapAesKey + decryptFileBody, adds extractFilesWithKeys and additionalKeys override support. |
| job-results/index.ts | Re-exports the new job-results/crypto API surface. |
| job-results/crypto.ts | New module containing unwrap/wrap and zip-free decrypt primitives. |
| job-results/crypto.test.ts | New tests for wrap/unwrap, decryptFile, and researcher override-key re-wrap flow. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- reader.decode now tracks whether manifest.json was found and throws on a real miss instead of silently using the empty default; use === for the filename check. - crypto.test selects the a.csv entry by path rather than index [0], removing dependence on zip entry ordering. Co-Authored-By: Claude Opus 4.8 <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.
Card 71 needs researchers to decrypt shared results without re-encryption. This adds the primitives the re-wrap flow depends on:
unwrapAesKey(exposes the raw AES bytes),wrapAesKey(re-wrap a file's key for another recipient),decryptFileBody, anddecryptFile(zip-free single-file decrypt returning{contents, rawAesKey}).ResultsReadergainsextractFilesWithKeys(returns each file's raw AES key for re-wrapping) and anadditionalKeysoverride map, letting a recipient absent from the original manifest (e.g. a researcher granted access later) decrypt the same ciphertext through the normal fingerprint path.ResultsReader/ResultsWriter are refactored onto these primitives with no behavior change. Wrap/unwrap, standalone decrypt, and the researcher override-key re-wrap round-trip are covered by crypto.test.ts.