feat(enrichment): changed-line coverage-delta analyzer (#1516#1637
feat(enrichment): changed-line coverage-delta analyzer (#1516#1637dale053 wants to merge 8 commits into
Conversation
| if (method === 0) { | ||
| data = compData; // stored — no decompression | ||
| } else if (method === 8) { | ||
| try { data = inflateRawSync(compData); } |
There was a problem hiding this comment.
P2: ZIP decompression bomb risk: inflateRawSync lacks output size limit
A compromised CI artifact with a high-ratio DEFLATE bomb can exhaust memory before the 2 MB cap is checked.
Pass maxOutputLength: MAX_COVERAGE_BYTES to inflateRawSync so decompression aborts early.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="review-enrichment/src/analyzers/coverage-delta.ts">
<violation number="1" location="review-enrichment/src/analyzers/coverage-delta.ts:184">
<priority>P2</priority>
<title>ZIP decompression bomb risk: inflateRawSync lacks output size limit</title>
<evidence>`readZipEntries` calls `inflateRawSync(compData)` at line 184 without passing a `maxOutputLength` option, then checks `data.length > MAX_COVERAGE_BYTES` only after the full buffer has been decompressed into memory. A compromised or attacker-influenced CI artifact with a high-ratio DEFLATE bomb could exhaust memory or crash the Node.js process before the size cap is enforced.</evidence>
<recommendation>Pass `maxOutputLength: MAX_COVERAGE_BYTES` to `inflateRawSync` so decompression aborts early if the output exceeds the 2 MB cap. Alternatively, use a streaming inflater and abort once the cap is reached.</recommendation>
</violation>
</file>
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review — held for maintainer review
⏸️ Held for maintainer review — Large change — held for manual review
Nits — 2 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
… in coverage-delta
Signed-off-by: dale053 <[email protected]>
Signed-off-by: dale053 <[email protected]>
…ock (JSONbored#1516) Three syntax defects flagged by the gate: - types.ts: CoverageDeltaFinding was missing its closing `}`, causing CodeownersFinding to be declared inside the prior interface body. - render.ts: the coverageDelta `for` loop and surrounding `if` block were unclosed before the codeownersViolations declaration, making renderBrief syntactically invalid. - enrichment.test.ts: the buildBrief/coverageDelta integration test was missing its `finally { globalThis.fetch = realFetch; }` and closing `});`, nesting all subsequent tests inside it and producing an EOF syntax error. All 92 enrichment tests pass after the fixes.
JSONbored
left a comment
There was a problem hiding this comment.
Similar to the last review I left, there are 4 merge conflicts due to previous analyzer merges - please resubmit after fixing..
This branch has conflicts that must be resolved
Use the [web editor](https://github.com/JSONbored/gittensory/pull/1637/conflicts) or the command line to resolve conflicts before continuing.
review-enrichment/src/brief.ts
review-enrichment/src/render.ts
review-enrichment/src/types.ts
review-enrichment/test/enrichment.test.ts
Summary
Adds a
coverageDeltaanalyzer to REES (#1516) that surfaces added/changed PR lines not covered by the project's own CI test suite, without requiring a repo checkout.coverage,lcov,cov-report, etc.), downloads its ZIP, and parses the first recognised coverage file inside it.lcov.info/*.lcov), Istanbul/NYC JSON (coverage-final.json), and Cobertura XML (coverage.xml/cobertura.xml).zlib.inflateRawSync(no new dependencies); the central-directory reader supports stored (method 0) and DEFLATE (method 8) entries.[\s\S]*?ReDoS patterns on attacker-influenced input.[]without throwing.promptSectionas a### Changed lines not covered by testsblock.Resolves #1516.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥97% coverage of the lines AND branches you changed (aim for 98%+ on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
test:workers— no Cloudflare Worker bindings touched; REES is a standalone Node.js service.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
No visible UI changes — REES is a backend enrichment service; output appears only in the review engine's prompt section.
Notes
/home/runner/work/repo/repo/src/foo.tsmatches PR filesrc/foo.ts) via a suffix check inpathMatches.makeStoredZiptest helper builds a minimal valid ZIP (stored compression, central directory + EOCD) entirely fromBuffer.alloc/writeUInt*— no fixture files needed.toArrayBufferslices the Buffer into a standaloneArrayBufferto avoid Node.js poolbyteOffsetissues when passing toBuffer.from(arrayBuffer).