Preserve review history across rebases

[wesm]

Summary

Adds rebase-aware review preservation so reviews survive git rebase and git commit --amend.

• Store patch_id (content hash via git patch-id --stable) at enqueue time to match reviews across rebases
• Add post-rewrite git hook that calls roborev remap after rebase/amend to update job refs
• Add /api/remap daemon endpoint with proper error responses (400 for non-git paths, 404 for unknown repos, 500 for DB errors)
• Add roborev remap CLI command that reads old/new SHA pairs from stdin, filters by matching patch-id, and sends mappings to the daemon
• Hook install/uninstall handles both post-commit and post-rewrite with append/upgrade/skip behavior
• SQLite and PostgreSQL migrations add patch_id column with index (schema v5)
• Sync support: patch_id round-trips through push/pull
• Input validation: SHA format checks on remap requests, repo_path validated before git resolution
• GetPatchID uses -c color.ui=false to prevent user git config from affecting patch-id computation

Test plan

• go test ./... passes
• go test -tags integration ./internal/daemon/ — rebase and amend integration tests pass
• roborev init installs post-rewrite hook alongside post-commit
• roborev install-hook appends to existing non-roborev hooks, upgrades outdated ones
• roborev uninstall-hook removes both post-commit and post-rewrite snippets
• Make a commit, review it, rebase, verify roborev show <new-sha> finds the review
• Same flow with git commit --amend (message-only change)
• Remap against non-git path returns 400, unregistered repo returns 404

Generated with Claude Code

[roborev-ci]

roborev: Combined Review

Verdict: Not ready to merge; 4 Medium-severity issues need fixes.

Medium

1. Unauthenticated state-changing remap endpoint

   • Location: internal/daemon/server.go:1349
   • Issue: POST /api/remap mutates review state without authz/authn checks; any local process that can reach the daemon can trigger remaps.
   • Fix: Require authentication/authorization (or equivalent local capability checks) for mutating endpoints.

2. Server trusts client-supplied remap integrity fields

   • Location: internal/daemon/server.go:1415, internal/storage/jobs.go:1509
   • Issue: old_sha/new_sha/patch_id and commit metadata are accepted from the client and used for remapping; legacy rows with patch_id IS NULL can be remapped without server-side proof.
   • Fix: Validate SHAs and commit existence server-side, recompute patch IDs on the server, and remap only on verified matches.

3. Shell expansion/injection risk in generated hook

   • Location: cmd/roborev/main.go:3515 (also reported around cmd/roborev/main.go:3510)
   • Issue: Hook content uses ROBOREV=%q (double-quoted shell string), which permits expansion/command substitution for crafted paths.
   • Fix: Use robust single-quote shell escaping for the executable path.

4. Legacy hook upgrade can leave duplicate roborev execution lines

   • Location: cmd/roborev/main.go:2403, cmd/roborev/main.go:2508, cmd/roborev/main.go:2529
   • Issue: Upgrade/removal logic misses older generated patterns, so old lines may remain and new snippet is appended, causing duplicate hook execution.
   • Fix: Broaden legacy pattern matching or remove generated hook blocks via marker-based block parsing.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[wesm]

I will rebase this after #278 goes in which has better review guidelines which will get the security review to chill out

[roborev-ci]

roborev: Combined Review

Verdict: Not ready to merge; the combined reviews indicate multiple unresolved security issues (2 High, 4 Medium).

High

• Untrusted remap data can tamper review history/metadata
  Refs: internal/daemon/server.go:1448, internal/storage/jobs.go:1519, internal/storage/jobs.go:1555
POST /api/remap accepts client-supplied old_sha/new_sha/patch_id/author/subject/timestamp and applies updates directly; empty/NULL patch_id handling allows remapping without proving commit equivalence. This enables history/metadata forgery.
  Fix: Recompute and verify patch IDs + commit metadata server-side from git objects, enforce SHA format, reject empty patch_id, and only remap on verified patch-ID matches.

• Git argument injection in patch-id path (git show)
  Refs: internal/git/git.go:928 (reachable from cmd/roborev/remap.go:45)
  User/script-controlled SHA can be interpreted as a git flag (option injection), potentially enabling file overwrite behaviors.
  Fix: Use end-of-options separation (for example --/--end-of-options) and validate SHA inputs before invoking git.

Medium

• Git argument injection in commit-info path (git log ... <sha>)
  Refs: internal/git/git.go:44 (reachable from cmd/roborev/remap.go:58)
  Same option-injection class as above for stdin-derived SHA values.
  Fix: Apply the same end-of-options + SHA-validation protections.

• No authz/authn on state-changing remap endpoint
  Refs: internal/daemon/server.go:89, internal/daemon/server.go:1406
POST /api/remap can be called by any local process (and potentially remote callers if bound beyond loopback).
  Fix: Require authenticated daemon access and add localhost CSRF/origin protections.

• Hook install/uninstall can follow symlinks/out-of-tree paths
  Refs: cmd/roborev/main.go:2391, cmd/roborev/main.go:2416, cmd/roborev/main.go:2538, cmd/roborev/main.go:2623, cmd/roborev/main.go:3467, cmd/roborev/main.go:3494
  Hook file read/write/delete operations lack symlink and path-boundary enforcement, allowing unintended file modification/removal.
  Fix: Lstat and reject symlinks/non-regular files, enforce resolved path under hooks dir, and use no-follow + atomic replace semantics.

• Missing strict SHA validation in remap input
  Refs: cmd/roborev/remap.go:45
oldSHA/newSHA are consumed directly from stdin without strict hash validation, expanding attack surface and malformed-input behavior.
  Fix: Validate against commit-hash format (or verify with rev-parse --verify <sha^{commit}>) before processing.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Changes are not ready to merge due to one Medium-severity security issue.

Medium

1. Argument injection in git show

• Location: internal/git/git.go:926 (GetPatchID), reachable from cmd/roborev/remap.go:45
• Issue: sha is passed directly as an argument (exec.Command("git", "show", sha)), so values beginning with - can be interpreted as Git flags instead of a revision.
• Impact: In the roborev remap stdin flow, crafted input can alter Git command behavior (for example, unintended option effects / file writes depending on flags).
• Recommended fix: Enforce strict SHA validation/resolution before use (for example via git rev-parse --verify) and pass revision after an end-of-options separator (for example git show -- <sha> where applicable).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: One medium-severity issue should be fixed before merge; no high or critical findings were identified.

Medium

1. Missing repo_path validation can remap the wrong repository

• Location: internal/daemon/server.go:1418
• Issue: handleRemap calls git.GetMainRepoRoot(req.RepoPath) without first validating req.RepoPath. If repo_path is empty, git resolution may fall back to the daemon process CWD, which can cause remap operations to target an unintended repo.
• Recommended fix: Reject empty/non-absolute repo_path with 400 before canonicalizing and proceeding with remap logic.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: PR has 3 actionable issues (1 High, 2 Medium) that should be addressed before merge.

High

• Argument injection risk in git invocation
  • Locations: internal/git/git.go:924, internal/git/git.go:928, internal/daemon/server.go:699
  • GetPatchID passes caller-influenced revision input directly to git show (exec.Command("git", "show", sha)), allowing option-like values to be interpreted by git and alter behavior.
  • Recommended fix: Strictly validate/resolve commit IDs before use (for example via git rev-parse --verify --end-of-options <rev>^{commit}), and only pass verified SHAs downstream.

Medium

• Potential local DoS in /api/remap due to unbounded request/body processing

  • Locations: internal/daemon/server.go:1410, internal/daemon/server.go:1452
  • Handler decodes JSON from r.Body without a size cap and processes unbounded req.Mappings with per-item DB work, enabling memory/CPU/DB contention from oversized requests.
  • Recommended fix: Add http.MaxBytesReader, cap mapping count (for example max 100), and enforce max field sizes (old_sha, new_sha, patch_id, author, subject) before DB operations.

• uninstall-hook can leave post-rewrite hook behind

  • Locations: cmd/roborev/main.go:2441, cmd/roborev/main.go:2451, cmd/roborev/main.go:2492
  • Early return when post-commit is missing/non-roborev prevents post-rewrite cleanup, so remap hook behavior can persist unexpectedly after uninstall.
  • Recommended fix: Always run post-rewrite cleanup regardless of post-commit state (for example move cleanup before early returns or ensure via defer).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: 3 Medium issues remain; no Critical or High findings were identified.

Medium

1. Shell expansion risk in generated git hook

   • Refs: cmd/roborev/main.go:3470, cmd/roborev/main.go:3516, cmd/roborev/main.go:3519
   • generatePostRewriteHookContent() uses ROBOREV=%q in a POSIX shell script. Go %q is not shell-safe quoting, so $... / $(...) can still expand if present in the path.
   • Suggested fix: Emit POSIX-safe single-quoted escaping (or avoid embedding path literals and resolve at runtime).

2. SHA validation blocks SHA-256 repos

   • Refs: cmd/roborev/remap.go:16, cmd/roborev/remap.go:51
   • Validation accepts only 40-hex SHAs, so valid 64-hex object IDs are dropped in SHA-256 repositories.
   • Suggested fix: Accept both 40 and 64 hex IDs (and add test coverage for 64-char SHAs).

3. GetPatchID lacks internal revision hardening

   • Ref: internal/git/git.go:928
   • exec.Command("git", "show", sha) relies on caller validation; if reused with untrusted input, revision/flag-style values can cause unsafe behavior.
   • Suggested fix: Harden in-function (git show -- <sha> and/or strict hex validation inside GetPatchID).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: 2 medium-severity issues need fixes before merge (no high/critical findings).

Medium

1. Hook removal/upgrade can fail silently, leaving hooks active

   • Reported by: codex (default)
   • Files/lines: cmd/roborev/main.go:2405, cmd/roborev/main.go:2441, cmd/roborev/main.go:2491, cmd/roborev/main.go:2570, cmd/roborev/main.go:2575
   • Issue: removeRoborevFromHook suppresses read/write/remove errors and returns no error, so uninstall/upgrade paths can report success even when cleanup failed.
   • Impact: Stale or duplicate hooks may continue running while users think uninstall/upgrade succeeded.
   • Suggested fix: Return and propagate errors (or at least emit explicit warnings with hook path + error) in uninstall/upgrade call paths.

2. Shell-unsafe quoting in generated post-rewrite hook

   • Reported by: gemini (default), codex (default), gemini (security)
   • Files/lines: cmd/roborev/main.go:3466, cmd/roborev/main.go:3468
   • Issue: ROBOREV=%q generates a double-quoted shell string; shell expansions like $()/`...` can still execute if present in the path.
   • Impact: Potential command execution from crafted executable paths.
   • Suggested fix: Use single-quote-safe shell escaping for the embedded path (or resolve executable at runtime instead of embedding).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Summary verdict: Mostly solid, but one Medium issue should be fixed before merge.

Medium

1. Legacy hook cleanup can leave old roborev commands active (duplicate execution risk)
   • cmd/roborev/main.go:2405
   • cmd/roborev/main.go:2502
   • cmd/roborev/main.go:2557
   • removeRoborevFromHook only treats $ROBOREV-style lines as snippet content. Older generated hooks used direct commands (for example, roborev enqueue --quiet &) without $ROBOREV. In that legacy format, the marker can be removed while command lines remain, and installOrUpgradeHook appends a new snippet, causing duplicate hook execution and incomplete uninstall/upgrade behavior.
   • Suggested fix: when inside a detected roborev marker block, also remove legacy direct forms (roborev enqueue ..., roborev remap ...) and add a regression test for that legacy format.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Two Medium issues should be addressed before merge; no High/Critical findings were reported.

Medium

1. Unbounded /api/remap input allows daemon DoS

   • Location: internal/daemon/server.go:1429
   • Reported by: codex (security)
   • Issue: handleRemap decodes directly from r.Body with no body-size cap and accepts an unbounded req.Mappings list, enabling excessive memory/CPU/DB work.
   • Recommended fix: Apply http.MaxBytesReader, enforce a maximum mapping count, and reject oversized requests (413/400).

2. Legacy hook uninstall/upgrade regression

   • Location: cmd/roborev/main.go:2463, cmd/roborev/main.go:2534
   • Reported by: codex (default)
   • Issue: removeRoborevFromHook only matches newer marker prefixes, so older hook variants may not be removed; uninstall can appear successful while leaving stale lines, and upgrade can duplicate blocks.
   • Recommended fix: Support legacy markers (or fallback matching for known older roborev hook lines) and add regression tests for legacy hook formats.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: 3 Medium issues found (no Critical/High findings).

Critical

None.

High

None.

Medium

1. Potential git argument injection via unvalidated git_ref

   • File/line: internal/daemon/server.go (around handleEnqueue, approx. line 713)
   • Details: req.GitRef-derived sha is passed into git.GetPatchID (git show <sha>) without strict boundary validation in this handler. A ref starting with - could be interpreted as a git flag.
   • Suggested fix: Validate accepted ref format at the API boundary (or reject leading -), and/or harden GetPatchID invocation semantics.

2. Hook upgrade/install can break non-shell hooks

   • File/line: cmd/roborev/main.go (installOrUpgradeHook append path; mirrored in installPostRewriteHook)
   • Details: Existing hook content is appended with roborev shell script content; if the existing hook is Python/Ruby/etc., resulting mixed script may fail execution.
   • Suggested fix: Detect hook interpreter/shebang before append; for non-shell hooks, fail with explicit manual-integration guidance (or require force overwrite behavior).

3. Hook cleanup/removal logic can leave roborev lines behind

   • File/line: cmd/roborev/main.go (removeRoborevFromHook, marker-based block detection)
   • Details: Removal starts from recognized marker lines only; markerless or multiple roborev blocks may survive uninstall/upgrade, causing duplicate behavior.
   • Suggested fix: Support repeated block removal and add fallback matching for known roborev command lines when marker detection misses.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: No Medium, High, or Critical issues identified across the combined reviews.

Consolidated Findings (Medium+)

No findings at Medium severity or above.

Notes

Security reviews from both agents reported no issues. The only concerns raised were Low severity and are omitted per requested threshold.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Changes are not yet ready to merge due to 5 unresolved Medium-severity issues in remap/auth/error-handling paths.

Medium

1. Unauthenticated mutating remap endpoint

   • Location: internal/daemon/server.go:86, internal/daemon/server.go:1418
   • Issue: POST /api/remap mutates state without authz/authn checks; any local process (and potentially localhost CSRF-style requests) can trigger remaps.
   • Fix: Require and verify a local capability/auth token (or equivalent) for mutating daemon APIs.

2. Missing server-side validation for remap identifiers

   • Location: internal/daemon/server.go:1459, internal/daemon/server.go:1474, internal/storage/jobs.go:1467
   • Issue: old_sha, new_sha, patch_id are not strictly validated at the HTTP trust boundary, enabling invalid/arbitrary values into remap persistence.
   • Fix: Enforce strict format/length validation server-side and reject invalid input with 400.

3. Remap handler masks storage/internal failures as “skipped”

   • Location: internal/daemon/server.go:1503
   • Issue: Per-mapping DB errors are swallowed and request still returns 200, hiding real failures.
   • Fix: Return server error for internal DB failures (or report them separately from true no-op skips).

4. CLI remap exits success even when daemon remap fails

   • Location: cmd/roborev/remap.go:101
   • Issue: Returning nil on daemon failure makes errors invisible to callers/scripts.
   • Fix: Return non-zero on failure (and keep hook behavior non-fatal via hook-side || true or a dedicated hook mode).

5. Potential deadlock in patch-id generation pipeline

   • Location: internal/git/git.go (inside GetPatchID)
   • Issue: If patchID.Start() fails and pipe reader is not closed, show may block on a full pipe and show.Wait() can hang.
   • Fix: Close the pipe in the patchID.Start() error path before waiting on show.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[wesm]

"⏺ Summary of the 5 findings:

1. Unauthenticated remap endpoint — Not valid. Explicitly covered by review guidelines: daemon is
   localhost-only, no auth needed.
2. Missing server-side SHA validation — Not valid. CLI validates with regex at the trust boundary;
   guidelines say don't re-flag at internal call sites.
3. Remap masks DB errors as "skipped" — Marginal validity. RemapJob returns (0, nil) for no-match
   and (0, error) for real failures, and real failures are silently swallowed. But the impact is low
   — DB errors during remap would be systemic and visible elsewhere. Not worth changing the API
   contract for this edge case.
4. CLI remap exits success on failure — Intentional. The return nil is explicitly commented "Don't
   fail the hook" — a non-zero exit from post-rewrite would print errors after every rebase.
5. Pipe deadlock in GetPatchID — Valid, fixed. Closing the pipe reader when patchID.Start() fails
   prevents show.Wait() from hanging if the diff exceeds the pipe buffer."

[roborev-ci]

roborev: Combined Review

Verdict: Changes are not clean; 2 medium-severity issues should be addressed before merge.

Medium

1. Potential arbitrary file overwrite via symlinked hook paths (deduplicated across reviews)

   • Location: cmd/roborev/main.go:2426, cmd/roborev/main.go:2432, cmd/roborev/main.go:3492, cmd/roborev/main.go:3499
   • Issue: Hook install/upgrade uses os.WriteFile under .git/hooks/* without rejecting symlinks. A malicious symlinked hook path could redirect writes to unintended files.
   • Recommended fix: Lstat before read/write/remove and reject symlinks; on supported platforms, open with no-follow semantics and ensure resolved paths stay within .git/hooks.

2. Hook upgrade may discard existing custom hook content on read errors

   • Location: cmd/roborev/main.go (upgrade path in installOrUpgradeHook; same pattern in installPostRewriteHook)
   • Issue: After removing old roborev content, re-read failures are ignored unless read succeeds, and the code can proceed to write fresh generated content only. Non-os.IsNotExist errors can therefore lead to unintended loss of user hook content.
   • Recommended fix: Treat readErr != nil && !os.IsNotExist(readErr) as a hard error (or abort write with warning) instead of continuing.

---

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly solid change set, but 3 Medium-severity issues should be addressed before merge.

Medium

1. Potential git argument injection path via unvalidated SHA

   • Files: internal/git/git.go:924, internal/daemon/server.go:713
   • GetPatchID runs git show <sha> via exec.Command, and handleEnqueue can pass request-provided sha into this path without enforcing strict SHA format at the helper boundary. A hyphen-prefixed value could be interpreted as a git flag.
   • Fix: Enforce SHA validation inside GetPatchID (preferred), or strictly validate in handleEnqueue before calling git helpers.

2. Hook uninstall/upgrade can remove user-authored lines

   • File: cmd/roborev/main.go:2520
   • The snippet-removal matcher is broad enough that custom user lines beginning with roborev enqueue / roborev remap immediately after the managed block may be deleted unintentionally.
   • Fix: Narrow matching to exact generated forms (and keep broad matching only for explicit legacy marker paths), plus add a regression test for preserving adjacent custom roborev ... lines.

3. Internal remap DB errors are masked as “skipped” with HTTP 200

   • File: internal/daemon/server.go:1490
   • Errors from db.RemapJob are treated as skipped results, which can hide operational failures (especially with quiet hook execution) and make remap breakage hard to detect.
   • Fix: Differentiate “no match” vs internal DB error, log internal errors, and return an error signal (5xx and/or explicit error counts).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review (ce981649)

Verdict: Mostly solid change set, but there are 2 Medium-severity correctness issues that should be addressed before merge.

Medium

1. Hook interpreter safety check can be bypassed

   • Files/lines: cmd/roborev/main.go:2394, cmd/roborev/main.go:468, cmd/roborev/main.go:3473
   • Issue: The hook update path checks strings.Contains(..., "roborev") before confirming the hook is actually a generated roborev shell hook. If a non-shell hook (e.g., Python/Ruby) contains the word roborev, the code can append/overwrite shell content and corrupt the hook.
   • Recommended fix: Only treat hooks as managed/generated when a real generated marker is present; otherwise route through the non-managed path with isShellHook validation (or require --force).

2. handleRemap masks database failures as skips

   • File/line: internal/daemon/server.go:1496
   • Issue: db.RemapJob errors are counted as skipped and still return HTTP 200, making real DB failures (locks/write/migration issues) indistinguishable from normal no-op skips.
   • Recommended fix: Surface DB errors as non-200 responses, or at least track/report a separate failed count and log failures distinctly.

No Critical or High findings were reported.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review (b2d51fc9)

Verdict: Not ready to merge — 1 High-severity issue needs to be fixed.

High

• internal/storage/jobs.go:786
  Issue: EnqueueJob has a SQL placeholder/argument count mismatch. The VALUES clause includes one more ? placeholder than the number of arguments passed to db.Exec (18 placeholders expected, 17 args provided).
  Impact: Runtime insert failure when enqueueing jobs on this path.
  Suggested fix: Remove the extra ? in the SQL VALUES list (or supply the missing argument) so placeholders and arguments match exactly.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review (8afaf4ea)

Verdict: Mostly solid change set, but 2 Medium-severity correctness/reliability issues should be fixed before merge.

Medium

1. Silent success on DB errors in remap API

   • Location: internal/daemon/server.go (handleRemap, loop calling s.db.RemapJob)
   • Issue: Transaction/storage errors are treated as skips and the handler still returns 200, which can silently lose remap operations on real DB failures.
   • Recommendation: Differentiate “no match” vs actual DB error. Return 500 for DB errors (or at minimum surface explicit per-item errors in the response and logs).

2. Potential hook overwrite on re-read failure

   • Location: cmd/roborev/main.go (installPostRewriteHook)
   • Issue: After removeRoborevFromHook, a non-ENOENT re-read failure may fall through to writing new hook content, risking overwrite of user-managed hook content.
   • Recommendation: Handle re-read errors the same way as installOrUpgradeHook (abort on non-ENOENT) or refactor to reuse shared logic and avoid drift.

---

Synthesized from 4 reviews (agents: gemini, codex | types: security, default)

[roborev-ci]

roborev: Combined Review (ee186355)

Verdict: Mostly solid change set, but one Medium correctness issue should be addressed before merge.

Medium

1. Non-shell hooks can still be corrupted if they merely mention roborev
   • Files: cmd/roborev/main.go:2400, cmd/roborev/main.go:2520
   • Issue: In installOrUpgradeHook, non-shell hooks can enter the “upgrade” path when they contain the string roborev even if they do not contain a generated roborev marker. removeRoborevFromHook then does nothing, and a shell snippet is appended to a non-shell hook.
   • Impact: Can silently break existing non-shell git hooks.
   • Suggested fix: Only treat as upgrade when a generated roborev marker is actually found/removed; otherwise, if the hook is non-shell, return an error instead of appending.

Notes

• Security-focused reviews reported no exploitable security issues in this diff.

---

Synthesized from 4 reviews (agents: gemini, codex | types: default, security)

[roborev-ci]

roborev: Combined Review (c5372d3e)

No Medium, High, or Critical issues found; this PR is clean at the requested reporting threshold.

All review outputs were consolidated and deduplicated. The only reported concerns were Low severity, so they are intentionally omitted per your rules.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)