ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

Review Summary by Qodo

(Agentic_describe updated until commit a83054c)

feat(reviews): /release-review periodic full-codebase review system with three-phase orchestration

✨ Enhancement 🧪 Tests 📝 Documentation

Walkthroughs

• Implements /release-review — a comprehensive periodic full-codebase review system for Claude
  Code, designed to run once per release version
• **Three-phase orchestrator architecture:**
  - Phase 1: Deterministic tools sweep (pnpm audit, osv-scanner, semgrep, eslint, madge,
  depcheck) via single subagent
  - Phase 2: Four parallel AI subagents audit assigned subsystems (auth/sessions, realtime-api,
  pad-changeset, db-supply-chain)
  - Phase 3: Aggregation, deduplication, suppression filtering, auto-triage classification, and
  interactive walkthrough
• **Core utilities in src/node/utils/releaseReview/:**
  - fingerprint.ts: Stable sha256 hashing over ruleId::file::5-line-context survives line-shift
  drift but breaks on logic changes
  - aggregate.ts: Merges findings, deduplicates by fingerprint, applies severity floor and
  suppression filtering
  - triage.ts: Auto-classifies findings into Fix-now/Issue/Suppress buckets using heuristics
  - suppression.ts: Loads/validates known-findings.yml with strict schema and date coercion for
  js-yaml
  - cli.ts: Orchestrates phases with commands: aggregate, triage, append-suppression,
  summary
  - runDir.ts, summary.ts: Run directory management and markdown summary generation
• **Comprehensive test coverage:** 53 unit tests validating fingerprint stability, aggregation
  deduplication, triage classification, suppression handling, and CLI commands
• **Documentation:** Design specification, implementation plan, operator guide, and five subagent
  prompts (tools, auth-sessions, realtime-api, pad-changeset, db-supply)
• **Dependencies:** Adds js-yaml@^4.1.1 as direct dependency for YAML parsing
• **Slash command:** .claude/commands/release-review.md orchestrates the full workflow with
  --resume support for continuing interrupted runs

flowchart LR
  Setup["Setup: Allocate run-id"]
  Phase1["Phase 1: Tools Sweep<br/>pnpm audit, osv-scanner,<br/>semgrep, eslint, madge, depcheck"]
  Phase2["Phase 2: AI Subagents<br/>auth-sessions, realtime-api,<br/>pad-changeset, db-supply"]
  Aggregate["Aggregate: Deduplicate<br/>by fingerprint, apply<br/>suppression & severity floor"]
  Triage["Triage: Auto-classify<br/>into Fix-now/Issue/Suppress"]
  Walkthrough["Walkthrough: Interactive<br/>decision & suppression update"]
  Summary["Summary: Generate<br/>markdown report"]
  
  Setup --> Phase1
  Phase1 --> Phase2
  Phase2 --> Aggregate
  Aggregate --> Triage
  Triage --> Walkthrough
  Walkthrough --> Summary

File Changes

Comprehensive unit tests for release-review utilities

• Comprehensive test suite with 53 unit tests covering fingerprint stability, aggregation, triage
 classification, run-dir management, summary generation, CLI commands, and suppression file handling
• Tests verify fingerprint survives whitespace-only line shifts but breaks on logic changes
• Tests validate aggregation deduplication by fingerprint, severity filtering, and sorting logic
• Tests confirm triage classification into fixNow/issue/suppress buckets based on severity,
 category, and remediation hints

src/tests/backend/specs/releaseReview-utils.ts

CLI orchestrator for release-review phases

• CLI entry point with commands: next-run-id, aggregate, triage, append-suppression,
 summary
• aggregate command reads JSON findings from run-dir, enriches missing fingerprints by computing
 from file content, applies suppression and severity floor, writes merged.json
• triage command classifies merged findings into buckets and writes triage.json
• append-suppression and summary commands support suppression file updates and markdown summary
 generation

src/node/utils/releaseReview/cli.ts

Suppression file loading and appending logic

• loadSuppression() parses and validates known-findings.yml with strict schema checking and date
 coercion for js-yaml Date objects
• appendSuppression() adds entries while preserving header comments by re-emitting only the
 findings list
• Validates required fields, status enum, and deferred-specific targetRelease requirement

src/node/utils/releaseReview/suppression.ts

Markdown summary generation for review sessions

• writeSummary() generates markdown summary with run-id, version, severity counts, and decisions
 grouped by action
• Supports decision actions: fix, issue, wontfix, accepted-risk, deferred, skip
• Formats decisions with file, ruleId, optional rationale and issue URL

src/node/utils/releaseReview/summary.ts

Finding aggregation and deduplication logic

• aggregate() merges findings from multiple sources, deduplicates by fingerprint keeping highest
 severity, applies suppression filtering
• Filters by severity floor and annotates deferred findings with firstSeen run-id
• Sorts by severity descending then category rank (cve > bug > perf > supply-chain > lint)

src/node/utils/releaseReview/aggregate.ts

Type definitions for release-review domain

• Defines core types: Severity, Category, SuppressionStatus, Finding, SuppressionEntry,
 TriageBuckets
• Finding includes source, fingerprint, severity, category, file, line, ruleId, message, optional
 remediationHint and firstSeen
• SuppressionEntry tracks fingerprint, status, decision metadata, and optional targetRelease for
 deferred items

src/node/utils/releaseReview/types.ts

Run directory naming and creation utilities

• todayIso() returns current date as YYYY-MM-DD in local timezone
• nextRunId() scans baseDir for existing run-YYYY-MM-DD-N directories and returns next available
 id
• ensureRunDir() creates run directory idempotently and returns absolute path

src/node/utils/releaseReview/runDir.ts

Stable fingerprint computation for findings

• computeFingerprint() generates stable sha256 hash from ruleId, file path, and 5-line context
 window (2 above + line + 2 below)
• Context lines are trimmed of whitespace before hashing to survive line-shift drift
• Fingerprint breaks when actual logic changes, ensuring real edits resurface findings

src/node/utils/releaseReview/fingerprint.ts

Auto-triage classification heuristics

• classify() auto-triages findings into fixNow/issue/suppress buckets using heuristics
• fixNow: findings with remediationHint; issue: high-severity AI findings without hints; suppress:
 lint category or medium tool findings without hints
• Heuristic is best-effort; user always confirms during walkthrough

src/node/utils/releaseReview/triage.ts

Test fixture for fingerprint stability validation

• Fixture file with sample TypeScript code used for fingerprint stability tests
• Contains intentional equality check (==) on line 6 seeded as a test finding
• Includes comments warning against casual edits that would break fingerprint test assertions

src/tests/backend/fixtures/releaseReview/sample-source.ts

Complete design specification for release-review system

• Comprehensive design document for /release-review slash command covering problem statement,
 goals, three-phase architecture
• Phase 1: deterministic tools sweep (pnpm audit, osv-scanner, semgrep, eslint, madge, depcheck)
• Phase 2: four parallel AI subagent sweeps (auth-sessions, realtime-api, pad-changeset, db-supply)
• Phase 3: aggregation, suppression, auto-triage, and interactive walkthrough with
 Fix-now/Issue/Suppress buckets
• Includes fingerprint stability design, suppression file schema, first-run UX, and operational
 concerns

docs/superpowers/specs/2026-05-09-release-review-design.md

Slash command orchestration and walkthrough flow

• Slash command implementation guide with five phases: setup, tool sweep, AI sweep,
 aggregation/triage/walkthrough, summary
• Phase 0 allocates run-id; Phase 1 runs tools subagent; Phase 2 dispatches four AI subagents in
 parallel
• Phase 3 aggregates findings, applies suppression, auto-triages, and walks user through
 Fix-now/Issue/Suppress buckets
• Supports --resume <run-id> to skip phases 1-2 and continue from existing run-dir

.claude/commands/release-review.md

Add js-yaml as direct dependency for YAML parsing

• Adds js-yaml@4.1.1 as direct dependency (was only transitive) for YAML parsing in suppression
 file handling
• Adds @types/js-yaml@4.0.9 as dev dependency for TypeScript type support
• Updates redis package snapshots to reflect new dependency structure

pnpm-lock.yaml

Auth-sessions subagent prompt for Phase 2

• Phase 2 subagent prompt for auditing authentication and session management subsystem
• Scopes to auth-related files: Session, AuthorManager, SecurityManager, auth handlers, hooks,
 security utilities
• Focuses on token leakage, session fixation, CSRF, timing-attack-prone comparisons, auth bypass via
 hooks, OIDC/SSO handling, cookie misconfiguration
• Severity rubric: High for exploitable auth bypass/token leakage; Medium for realistic race
 conditions or hardening gaps

docs/reviews/prompts/auth-sessions.md

DB and supply-chain subagent prompt for Phase 2

• Phase 2 subagent prompt for auditing DB layer and supply-chain surface (CI, Docker, packaging)
• Scopes to DB managers, Dockerfile, GitHub Actions workflows, bin scripts, package files, snap/deb
 packaging
• Focuses on DB key injection, key collision risks, untrusted plugin paths, GitHub Actions
 injection, Dockerfile hygiene, release pipeline, lockfile drift
• Severity rubric: High for RCE on CI or supply-chain compromise; Medium for DB key collisions or
 unpinned actions

docs/reviews/prompts/db-supply.md

Tools sweep subagent prompt for Phase 1

• Phase 1 subagent prompt for deterministic tool sweep (pnpm audit, osv-scanner, semgrep, eslint,
 madge, depcheck)
• Defines normalized finding schema with source, severity, category, file, line, ruleId, message,
 remediationHint
• Specifies severity and category mappings per tool; explicitly excludes fingerprint computation
 (deferred to aggregate stage)
• Caps output at 30 findings; records tool errors for missing/failed tools

docs/reviews/prompts/tools.md

Operator guide and documentation for release-review

• Operator guide for /release-review with links to design, implementation plan, slash command, and
 helper modules
• Documents when to run (once per release), how to run (slash command or --resume), and what it
 produces (walkthrough, suppression file, summary)
• Explains suppression file schema and re-triaging workflow; documents first-run baseline-acceptance
 UX
• Includes smoke test checklist and guidance for updating prompts and adding new subsystems

docs/reviews/README.md

Realtime API subagent prompt for Phase 2

• Phase 2 subagent prompt for auditing realtime (socket.io) and HTTP API surface
• Scopes to PadMessageHandler, SocketIO handlers, API handlers, admin endpoints, rate-limit config
• Focuses on message validation gaps, race conditions, rate-limit bypasses, IDOR, broadcast leaks,
 unbounded growth/DoS, admin surface, spec drift
• Severity rubric: High for auth bypass/RCE/full DoS/IDOR; Medium for realistic race conditions or
 ReDoS

docs/reviews/prompts/realtime-api.md

Pad-changeset subagent prompt for Phase 2

• Phase 2 subagent prompt for auditing core editing engine (changesets, attribute pool, pad/revision
 storage)
• Scopes to Changeset, AttributePool, Pad, PadManager, and related utilities
• Focuses on changeset validation flaws, attribute pool exhaustion, revision integrity, unbounded
 growth, OT correctness, concurrency races, storage key safety
• Severity rubric: High for data corruption or RCE; Medium for unrecoverable pad states or unbounded
 growth under attacker input

docs/reviews/prompts/pad-changeset.md

Add js-yaml dependencies to package.json

• Adds js-yaml@^4.1.1 to dependencies for YAML parsing in suppression file handling
• Adds @types/js-yaml@^4.0.9 to devDependencies for TypeScript type support

src/package.json

Valid suppression file test fixture

• Valid suppression file fixture with two entries: one wontfix and one deferred with
 targetRelease
• Used by tests to verify correct parsing and validation of suppression entries

src/tests/backend/fixtures/releaseReview/suppression-valid.yml

Initial suppression file for release-review

• Initial suppression file for /release-review with empty findings list
• Includes header comments documenting manual editing guidelines and schema reference
• Starts empty; entries appended automatically by /release-review during triage

docs/reviews/known-findings.yml

Malformed YAML test fixture

• Malformed YAML fixture with unterminated string to test error handling in suppression file parsing

src/tests/backend/fixtures/releaseReview/suppression-malformed.yml

Bad-shape suppression file test fixture

• Suppression file fixture with missing required fields (fingerprint) to test schema validation

src/tests/backend/fixtures/releaseReview/suppression-bad-shape.yml

Complete implementation plan for /release-review orchestrator

• Comprehensive implementation plan for the /release-review slash command with 17 sequential tasks
• Defines file structure, directory layout, and module responsibilities for the release review
 system
• Includes detailed step-by-step instructions for implementing TypeScript helpers, CLI entry point,
 and subagent prompts
• Provides TDD-style test specifications and integration smoke tests for end-to-end verification

docs/superpowers/plans/2026-05-09-release-review.md

...

docs/reviews/prompts/.gitkeep

...

src/node/utils/releaseReview/.gitkeep

...

src/tests/backend/fixtures/releaseReview/.gitkeep

Code Review by Qodo

🐞 Bugs (6) 📘 Rule violations (0)

Description

computeFingerprint() hashes only the trimmed context window; if the file is missing/unreadable (or
the reported line is far out of range such that the slice is empty), the context becomes empty and
all such findings collapse to the same fingerprint for a given ruleId+file. This breaks
dedupe/suppression by merging unrelated findings into one.

Code

src/node/utils/releaseReview/fingerprint.ts[R25-30]

+  const idx = line - 1;
+  const start = Math.max(0, idx - 2);
+  const end = Math.min(lines.length, idx + 3);
+  const context = lines.slice(start, end).map((l) => l.trim()).join('\n');
+  const payload = `${ruleId}::${file}::${context}`;
+  return createHash('sha256').update(payload).digest('hex');

Evidence

The CLI explicitly returns an empty lines array when the file does not exist, and
computeFingerprint() uses only the (possibly empty) context slice (excluding the line number), so
missing/unreadable files produce the same ruleId::file:: payload for all such findings.

src/node/utils/releaseReview/cli.ts[35-44]
src/node/utils/releaseReview/fingerprint.ts[25-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`computeFingerprint()` can produce identical fingerprints when it cannot extract any context (e.g., file missing/unreadable => `lines=[]`, or `start>=end`), because the payload becomes `${ruleId}::${file}::`.

## Issue Context
This happens today because the CLI’s `readLines()` returns an empty array when a file doesn’t exist, and `computeFingerprint()` does not add any other distinguishing input when the context slice is empty.

## Fix Focus Areas
- src/node/utils/releaseReview/fingerprint.ts[25-30]

## Implementation notes
- Detect the “no context” case (`lines.length===0` or `start>=end`).
- In that case, incorporate a deterministic fallback into the payload (e.g., include `line` and a sentinel like `__missing_file__` / `__out_of_range__:${line}/${lines.length}`) so distinct findings don’t collide.
- Keep the existing context-based behavior when context is present, to preserve the intended line-shift stability.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

aggregate() dedupes by fingerprint but chooses the merged representative only by severity; when
severities tie, the chosen category/message/remediationHint becomes order-dependent and can
downgrade a CVE-category finding to bug/perf/etc. This mis-prioritizes findings and can push
security-relevant items into the wrong downstream triage/sort order.

Code

src/node/utils/releaseReview/aggregate.ts[R38-45]

+        const winner = SEVERITY_RANK[annotated.severity] > SEVERITY_RANK[existing.severity]
+          ? annotated
+          : existing;
+        const sources = new Set([existing.source, annotated.source].flatMap((s) => s.split(',')));
+        byFingerprint.set(annotated.fingerprint, {
+          ...winner,
+          source: [...sources].join(','),
+        });

Evidence

The code defines a category ranking but the dedupe merge logic only compares severity; for equal
severity it always keeps the existing finding’s fields, making the resulting category dependent on
input order.

src/node/utils/releaseReview/aggregate.ts[5-7]
src/node/utils/releaseReview/aggregate.ts[38-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When merging two findings with the same fingerprint, `aggregate()` selects a winner based only on severity. If the severities are equal but categories differ (e.g., `cve` vs `bug`), the result keeps whichever category appeared first, even though a category rank exists.

## Issue Context
`CATEGORY_RANK` is defined and used for sorting, but not for deciding which finding’s fields to keep during dedupe.

## Fix Focus Areas
- src/node/utils/releaseReview/aggregate.ts[38-45]

## Implementation notes
- Update the winner selection to:
 - Prefer higher severity.
 - If equal severity, prefer higher `CATEGORY_RANK`.
- Consider also merging/selecting other key fields deterministically (e.g., prefer a non-empty `remediationHint`, and possibly keep the “most informative” `message`) so the merged record is not order-dependent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The aggregate CLI reads file contents for fingerprinting using the file field from subagent JSON,
allowing absolute paths and .. segments without verifying the resolved path stays inside
repoRoot. A malformed/malicious findings JSON can cause the CLI to read arbitrary host files when
computing fingerprints.

Code

src/node/utils/releaseReview/cli.ts[R35-50]

+    const readLines = (file: string): string[] => {
+      const abs = path.isAbsolute(file) ? file : path.join(repoRoot, file);
+      if (!fileLineCache.has(abs)) {
+        fileLineCache.set(
+          abs,
+          fs.existsSync(abs) ? fs.readFileSync(abs, 'utf8').split('\n') : [],
+        );
+      }
+      return fileLineCache.get(abs)!;
+    };
+    const enrich = (raw: any): Finding => {
+      // Subagent JSON may be top-level array OR {findings: [...]}.
+      if (raw.fingerprint) return raw;
+      const lines = readLines(raw.file);
+      const fp = computeFingerprint(raw.ruleId, raw.file, raw.line, lines);
+      return {...raw, fingerprint: fp};

Evidence

readLines() uses path.isAbsolute(file) ? file : path.join(repoRoot, file) and then reads that
path from disk; the path comes from subagent-emitted JSON and is used during fingerprint enrichment.
There is no path.resolve() + repoRoot containment check, so ../../.. or /etc/passwd-style
values are reachable.

src/node/utils/releaseReview/cli.ts[31-50]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` reads `raw.file` from subagent JSON and uses it to read file contents for fingerprint computation. Because the path is not normalized/validated, findings can reference absolute paths or use `..` to escape `repoRoot`, leading to arbitrary file reads.
## Issue Context
Even if subagents are “internal”, their output is effectively untrusted input. The aggregator should defensively ensure it only reads files inside the repository root.
## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[31-50]
## Suggested fix
- Resolve paths via `path.resolve()`.
- Compute `repoRootAbs = path.resolve(repoRoot)` once.
- For each `raw.file`:
- `abs = path.resolve(repoRootAbs, raw.file)` (even if `raw.file` is absolute, consider rejecting it or normalizing it to a repo-relative path)
- Reject if `path.relative(repoRootAbs, abs).startsWith('..'+path.sep)` or equals `..` (escape attempt).
- Use the validated `abs` for reading, and use a normalized repo-relative (POSIX) path for fingerprinting/suppression stability.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

Fingerprint computation includes the file string verbatim, and the CLI passes raw.file through
unchanged when computing the fingerprint; if any producer emits an absolute path, the same logical
finding will fingerprint differently across machines or when mixed with repo-relative producers.
This breaks cross-run suppression/dedupe even when the underlying code context is identical.

Code

src/node/utils/releaseReview/cli.ts[R48-50]

+      const lines = readLines(raw.file);
+      const fp = computeFingerprint(raw.ruleId, raw.file, raw.line, lines);
+      return {...raw, fingerprint: fp};

Evidence

The fingerprint payload explicitly includes the file string, the CLI uses raw.file in the
fingerprint input, and the test suite demonstrates that absolute file paths are accepted—meaning
fingerprints can vary with the local checkout path.

src/node/utils/releaseReview/fingerprint.ts[28-30]
src/node/utils/releaseReview/cli.ts[48-50]
docs/reviews/prompts/tools.md[36-53]
src/tests/backend/specs/releaseReview-utils.ts[329-346]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Fingerprints hash `${ruleId}::${file}::${context}` and `cli.ts aggregate` currently passes `raw.file` directly into the hash. If a tool/subagent outputs an absolute path, fingerprints become environment-specific and won’t match suppressions produced on another machine.

## Issue Context
Prompts specify `file` should be repo-relative, but the CLI/tests allow absolute paths, so the aggregator should normalize to one canonical format before hashing.

## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[35-50]
- src/node/utils/releaseReview/fingerprint.ts[28-30]

## Implementation notes
- In `enrich()` (aggregate command), compute a normalized repo-relative path when possible:
 - Resolve `abs` for reading as today.
 - If `abs` is under `repoRoot`, compute `rel = path.relative(repoRoot, abs)` and use `rel` (preferably with POSIX separators) as the `file` string passed to `computeFingerprint()`.
 - Optionally also replace the stored `file` field in the returned finding with the normalized repo-relative path to keep outputs consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The aggregator’s enrich() returns findings unchanged when raw.fingerprint is present, allowing
subagents/tools to override the deterministic fingerprint computation. This breaks the documented
contract (“no fingerprint — added by aggregate stage”) and can poison dedupe/suppression if a
subagent emits stale/differently-normalized fingerprints.

Code

src/node/utils/releaseReview/cli.ts[R45-50]

+    const enrich = (raw: any): Finding => {
+      // Subagent JSON may be top-level array OR {findings: [...]}.
+      if (raw.fingerprint) return raw;
+      const lines = readLines(raw.file);
+      const fp = computeFingerprint(raw.ruleId, raw.file, raw.line, lines);
+      return {...raw, fingerprint: fp};

Evidence

enrich() short-circuits on raw.fingerprint, skipping computeFingerprint(...). The subagent
prompt explicitly specifies that findings must not include fingerprints and that the aggregate stage
adds them, so accepting fingerprints makes the pipeline dependent on subagent behavior and
normalization choices.

src/node/utils/releaseReview/cli.ts[45-51]
docs/reviews/prompts/auth-sessions.md[52-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` currently accepts `raw.fingerprint` from input JSON. This bypasses deterministic fingerprint computation and violates the documented contract that fingerprints are added by the aggregator.
## Issue Context
A single subagent/tool emitting a stale or differently-normalized fingerprint can cause:
- incorrect deduplication across sources
- suppression mismatches across runs
## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[45-51]
- docs/reviews/prompts/auth-sessions.md[52-65]
## Suggested fix
- Change `enrich()` to always compute the fingerprint from current file contents.
- If `raw.fingerprint` is present, either:
- reject with a clear error (strict contract enforcement), or
- recompute and overwrite, optionally warning if the provided fingerprint differs.
- While at it, validate required fields (`file`, `line`, `ruleId`) before computing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The CLI casts the severityFloor argument to Severity without validation. If an invalid string is
passed, severity filtering can silently drop all findings because SEVERITY_RANK[floor] becomes
undefined.

Code

src/node/utils/releaseReview/cli.ts[R59-61]

+    const sup = loadSuppression(supPath);
+    const merged = aggregate(findingsArrays, sup, floor as Severity);
+    fs.writeFileSync(path.join(runDir, 'merged.json'), JSON.stringify(merged, null, 2));

Evidence

cli.ts passes floor as Severity into aggregate(). aggregate.ts then indexes
SEVERITY_RANK[floor] without guarding, so unexpected strings yield undefined and cause
meetsFloor() to return false for all findings.

src/node/utils/releaseReview/cli.ts[59-61]
src/node/utils/releaseReview/aggregate.ts[5-10]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` does not validate the `severityFloor` argument and casts it to `Severity`. Invalid values can result in all findings being filtered out.
## Issue Context
This is easy to hit via manual invocation or future script changes.
## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[31-63]
- src/node/utils/releaseReview/aggregate.ts[5-10]
## Suggested fix
- Add a small validator in `cli.ts` before calling `aggregate()`, e.g. ensure `floor` is one of `high|medium|low|info` and `die()` otherwise.
- (Optional) also validate `f.severity` values when aggregating to avoid NaN/undefined rank behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit a83054c

🐞 Bugs (3) 📘 Rule violations (0)

Description

The aggregate CLI reads file contents for fingerprinting using the file field from subagent JSON,
allowing absolute paths and .. segments without verifying the resolved path stays inside
repoRoot. A malformed/malicious findings JSON can cause the CLI to read arbitrary host files when
computing fingerprints.

Code

src/node/utils/releaseReview/cli.ts[R35-50]

+    const readLines = (file: string): string[] => {
+      const abs = path.isAbsolute(file) ? file : path.join(repoRoot, file);
+      if (!fileLineCache.has(abs)) {
+        fileLineCache.set(
+          abs,
+          fs.existsSync(abs) ? fs.readFileSync(abs, 'utf8').split('\n') : [],
+        );
+      }
+      return fileLineCache.get(abs)!;
+    };
+    const enrich = (raw: any): Finding => {
+      // Subagent JSON may be top-level array OR {findings: [...]}.
+      if (raw.fingerprint) return raw;
+      const lines = readLines(raw.file);
+      const fp = computeFingerprint(raw.ruleId, raw.file, raw.line, lines);
+      return {...raw, fingerprint: fp};

Evidence

readLines() uses path.isAbsolute(file) ? file : path.join(repoRoot, file) and then reads that
path from disk; the path comes from subagent-emitted JSON and is used during fingerprint enrichment.
There is no path.resolve() + repoRoot containment check, so ../../.. or /etc/passwd-style
values are reachable.

src/node/utils/releaseReview/cli.ts[31-50]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` reads `raw.file` from subagent JSON and uses it to read file contents for fingerprint computation. Because the path is not normalized/validated, findings can reference absolute paths or use `..` to escape `repoRoot`, leading to arbitrary file reads.

## Issue Context
Even if subagents are “internal”, their output is effectively untrusted input. The aggregator should defensively ensure it only reads files inside the repository root.

## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[31-50]

## Suggested fix
- Resolve paths via `path.resolve()`.
- Compute `repoRootAbs = path.resolve(repoRoot)` once.
- For each `raw.file`:
 - `abs = path.resolve(repoRootAbs, raw.file)` (even if `raw.file` is absolute, consider rejecting it or normalizing it to a repo-relative path)
 - Reject if `path.relative(repoRootAbs, abs).startsWith('..'+path.sep)` or equals `..` (escape attempt).
 - Use the validated `abs` for reading, and use a normalized repo-relative (POSIX) path for fingerprinting/suppression stability.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The aggregator’s enrich() returns findings unchanged when raw.fingerprint is present, allowing
subagents/tools to override the deterministic fingerprint computation. This breaks the documented
contract (“no fingerprint — added by aggregate stage”) and can poison dedupe/suppression if a
subagent emits stale/differently-normalized fingerprints.

Code

src/node/utils/releaseReview/cli.ts[R45-50]

+    const enrich = (raw: any): Finding => {
+      // Subagent JSON may be top-level array OR {findings: [...]}.
+      if (raw.fingerprint) return raw;
+      const lines = readLines(raw.file);
+      const fp = computeFingerprint(raw.ruleId, raw.file, raw.line, lines);
+      return {...raw, fingerprint: fp};

Evidence

enrich() short-circuits on raw.fingerprint, skipping computeFingerprint(...). The subagent
prompt explicitly specifies that findings must not include fingerprints and that the aggregate stage
adds them, so accepting fingerprints makes the pipeline dependent on subagent behavior and
normalization choices.

src/node/utils/releaseReview/cli.ts[45-51]
docs/reviews/prompts/auth-sessions.md[52-65]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` currently accepts `raw.fingerprint` from input JSON. This bypasses deterministic fingerprint computation and violates the documented contract that fingerprints are added by the aggregator.

## Issue Context
A single subagent/tool emitting a stale or differently-normalized fingerprint can cause:
- incorrect deduplication across sources
- suppression mismatches across runs

## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[45-51]
- docs/reviews/prompts/auth-sessions.md[52-65]

## Suggested fix
- Change `enrich()` to always compute the fingerprint from current file contents.
- If `raw.fingerprint` is present, either:
 - reject with a clear error (strict contract enforcement), or
 - recompute and overwrite, optionally warning if the provided fingerprint differs.
- While at it, validate required fields (`file`, `line`, `ruleId`) before computing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The CLI casts the severityFloor argument to Severity without validation. If an invalid string is
passed, severity filtering can silently drop all findings because SEVERITY_RANK[floor] becomes
undefined.

Code

src/node/utils/releaseReview/cli.ts[R59-61]

+    const sup = loadSuppression(supPath);
+    const merged = aggregate(findingsArrays, sup, floor as Severity);
+    fs.writeFileSync(path.join(runDir, 'merged.json'), JSON.stringify(merged, null, 2));

Evidence

cli.ts passes floor as Severity into aggregate(). aggregate.ts then indexes
SEVERITY_RANK[floor] without guarding, so unexpected strings yield undefined and cause
meetsFloor() to return false for all findings.

src/node/utils/releaseReview/cli.ts[59-61]
src/node/utils/releaseReview/aggregate.ts[5-10]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cli.ts aggregate` does not validate the `severityFloor` argument and casts it to `Severity`. Invalid values can result in all findings being filtered out.

## Issue Context
This is easy to hit via manual invocation or future script changes.

## Fix Focus Areas
- src/node/utils/releaseReview/cli.ts[31-63]
- src/node/utils/releaseReview/aggregate.ts[5-10]

## Suggested fix
- Add a small validator in `cli.ts` before calling `aggregate()`, e.g. ensure `floor` is one of `high|medium|low|info` and `die()` otherwise.
- (Optional) also validate `f.severity` values when aggregating to avoid NaN/undefined rank behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

Persistent review updated to latest commit a83054c