From 00542a15e4636cc5483aefec68afbc554bf5593a Mon Sep 17 00:00:00 2001
From: McAtk <16798627+atk0309@users.noreply.github.com>
Date: Thu, 23 Jul 2026 21:18:15 +0100
Subject: [PATCH] fix: resolve CodeQL baseline alerts

---
 backend/app/auth/magic.py                    |  6 +-
 backend/app/persistence/application_store.py | 26 +++++++--
 backend/app/routes/cv.py                     |  8 ++-
 backend/tests/test_auth.py                   | 22 +++++++-
 docs/agent-guidance-changelog.md             | 10 ++++
 docs/architecture/renderer.md                |  2 +
 docs/architecture/security.md                | 11 +++-
 docs/testing/strategy.md                     | 12 +++-
 frontend/src/lib/bootstrap-scripts.test.ts   | 13 +++++
 frontend/src/lib/sidebar.ts                  |  7 ++-
 frontend/src/lib/theme.ts                    |  7 ++-
 frontend/src/render/fetch.test.ts            | 15 +++--
 frontend/src/render/fetch.ts                 | 13 ++++-
 scripts/check-doc-impact.mjs                 | 58 +++++++++++++-------
 scripts/check-doc-impact.test.mjs            |  7 +++
 15 files changed, 175 insertions(+), 42 deletions(-)
 create mode 100644 frontend/src/lib/bootstrap-scripts.test.ts

diff --git a/backend/app/auth/magic.py b/backend/app/auth/magic.py
index 90df431..cdd3e62 100644
--- a/backend/app/auth/magic.py
+++ b/backend/app/auth/magic.py
@@ -35,7 +35,11 @@ def read_magic_token(token: str) -> str | None:
 async def send_magic_email(*, to: str, link: str) -> None:
     """Email the sign-in link via Resend. Without a key (dev), log the link instead of sending."""
     if not settings.resend_api_key:
-        log.warning("RESEND_API_KEY unset — magic link for %s: %s", to, link)
+        log.warning(
+            "RESEND_API_KEY unset — magic link for %s: %s",
+            to.replace("\r", "").replace("\n", ""),
+            link.replace("\r", "").replace("\n", ""),
+        )
         return
     async with httpx.AsyncClient(timeout=10.0) as client:
         resp = await client.post(
diff --git a/backend/app/persistence/application_store.py b/backend/app/persistence/application_store.py
index 5efa6de..654547a 100644
--- a/backend/app/persistence/application_store.py
+++ b/backend/app/persistence/application_store.py
@@ -167,7 +167,12 @@ async def apply(
         # serializes the two, so this is the SQLite/no-advisory-lock backstop.)
         await session.rollback()
         raise AlreadyApplied(cv_id) from exc
-    log.info("application created: %s cv=%s ws=%s", application.id, cv_id, workspace_id)
+    log.info(
+        "application created: %s cv=%s ws=%s",
+        application.id.replace("\r", "").replace("\n", ""),
+        cv_id.replace("\r", "").replace("\n", ""),
+        workspace_id.replace("\r", "").replace("\n", ""),
+    )
     return application
 
 
@@ -235,7 +240,11 @@ async def update_stage(
     row.application = application.model_dump(mode="json", by_alias=True)
     await session.commit()
     log.info(
-        "application stage change: %s %s -> %s ws=%s", application_id, prev, stage, workspace_id
+        "application stage change: %s %s -> %s ws=%s",
+        application_id.replace("\r", "").replace("\n", ""),
+        prev.replace("\r", "").replace("\n", ""),
+        stage.replace("\r", "").replace("\n", ""),
+        workspace_id.replace("\r", "").replace("\n", ""),
     )
     return application
 
@@ -257,7 +266,11 @@ async def update_recruiter(
     application.recruiter_email = recruiter_email
     row.application = application.model_dump(mode="json", by_alias=True)
     await session.commit()
-    log.info("application recruiter updated: %s ws=%s", application_id, workspace_id)
+    log.info(
+        "application recruiter updated: %s ws=%s",
+        application_id.replace("\r", "").replace("\n", ""),
+        workspace_id.replace("\r", "").replace("\n", ""),
+    )
     return application
 
 
@@ -287,4 +300,9 @@ async def delete_application(session: AsyncSession, workspace_id: str, applicati
     # in the SAME commit under the lock. delete_review rides this transaction (no inner commit).
     await review_store.delete_review(session, workspace_id, cv_id)
     await session.commit()
-    log.info("application deleted: %s cv=%s ws=%s", application_id, cv_id, workspace_id)
+    log.info(
+        "application deleted: %s cv=%s ws=%s",
+        application_id.replace("\r", "").replace("\n", ""),
+        cv_id.replace("\r", "").replace("\n", ""),
+        workspace_id.replace("\r", "").replace("\n", ""),
+    )
diff --git a/backend/app/routes/cv.py b/backend/app/routes/cv.py
index a99adbb..5e85475 100644
--- a/backend/app/routes/cv.py
+++ b/backend/app/routes/cv.py
@@ -419,7 +419,10 @@ async def read_review(
     # orphan row). Never serve a review for a CV that no longer exists — self-heal by deleting
     # the orphan and reading as absent.
     if not await cv_store.cv_exists(session, workspace_id, cv_id):
-        log.warning("review: orphan review row for deleted cv=%s; removing", cv_id)
+        log.warning(
+            "review: orphan review row for deleted cv=%s; removing",
+            cv_id.replace("\r", "").replace("\n", ""),
+        )
         await review_store.delete_review(session, workspace_id, cv_id)
         await session.commit()
         return None
@@ -435,7 +438,8 @@ async def read_review(
         )
     except ValidationError:
         log.warning(
-            "review: stored review for cv=%s no longer validates; treating as absent", cv_id
+            "review: stored review for cv=%s no longer validates; treating as absent",
+            cv_id.replace("\r", "").replace("\n", ""),
         )
         return None
 
diff --git a/backend/tests/test_auth.py b/backend/tests/test_auth.py
index 14b2dd3..8a521d2 100644
--- a/backend/tests/test_auth.py
+++ b/backend/tests/test_auth.py
@@ -5,9 +5,11 @@
 
 from __future__ import annotations
 
+import asyncio
+
 import pytest
 from app.auth.allowlist import is_allowlisted
-from app.auth.magic import make_magic_token, read_magic_token
+from app.auth.magic import make_magic_token, read_magic_token, send_magic_email
 from app.auth.session import get_session_user, login_user
 from app.config import settings
 from app.main import app
@@ -32,6 +34,24 @@ def test_magic_token_roundtrip() -> None:
     assert read_magic_token("not-a-real-token") is None
 
 
+def test_dev_magic_link_log_strips_line_breaks(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    monkeypatch.setattr(settings, "resend_api_key", "")
+    with caplog.at_level("WARNING", logger="contender.auth"):
+        asyncio.run(
+            send_magic_email(
+                to="owner@example.com\r\nforged-recipient",
+                link="https://example.test/magic\r\nforged-entry",
+            )
+        )
+    message = caplog.records[-1].getMessage()
+    assert "\r" not in message
+    assert "\n" not in message
+    assert "forged-recipient" in message
+    assert "forged-entry" in message
+
+
 def test_me_requires_session() -> None:
     client = TestClient(app, base_url="https://testserver")
     assert client.get("/auth/me").status_code == 401
diff --git a/docs/agent-guidance-changelog.md b/docs/agent-guidance-changelog.md
index b9698d7..da41b91 100644
--- a/docs/agent-guidance-changelog.md
+++ b/docs/agent-guidance-changelog.md
@@ -3,6 +3,16 @@
 Durable changes to `AGENTS.md`, nested agent guidance, `CLAUDE.md`, repository scripts, and GitHub
 workflows are recorded here. The local `guidance-self-update` rule enforces that contract.
 
+## 2026-07-23 (CodeQL alert remediation)
+
+- Replaced the doc-impact checker's shell command strings with argument-vector `git` subprocesses
+  and rejected option-like refs before Git sees them.
+- Kept the security-extended CodeQL workflow strict: the first baseline alerts are fixed in source
+  rather than hidden by weakening the query suite or dismissing actionable results.
+
+Why for future agents: pass subprocess arguments as arrays, keep executable bootstrap strings
+static, and remove line breaks from user-derived values before writing them to plain-text logs.
+
 ## 2026-07-23 (public repository health hardening)
 
 - Added the missing CodeQL workflow for Python and JavaScript/TypeScript so the active `main`
diff --git a/docs/architecture/renderer.md b/docs/architecture/renderer.md
index 7993b36..44ab575 100644
--- a/docs/architecture/renderer.md
+++ b/docs/architecture/renderer.md
@@ -36,6 +36,8 @@ renders a clear *needs-sign-off blocked state* instead of the document — so a
 exported, only previewed (via the un-gated `GET /workspaces/{ws}/cv/{id}` preview bundle). An
 unreachable backend (bad `BACKEND_INTERNAL_URL` / ECONNREFUSED) is caught and surfaced as a clean
 `502` (mirroring `lib/proxy.ts`), so the export routes fail consistently rather than throwing a 500.
+Its diagnostic uses a constant format string and strips line separators from the workspace, CV, and
+error fields; the forwarded session cookie is never logged.
 
 **Synthetic sample only.** `frontend/src/render/fixtures.ts` and its tests use a clearly fictional
 candidate, `example.com` links, an invalid example country code, and synthetic contact details. The
diff --git a/docs/architecture/security.md b/docs/architecture/security.md
index 7c670d7..0c1bbae 100644
--- a/docs/architecture/security.md
+++ b/docs/architecture/security.md
@@ -1,7 +1,7 @@
 # Security
 
 Status: active — seed doc  
-Last updated: 2026-07-22
+Last updated: 2026-07-23
 
 Update on any change under `backend/app/auth/` (`auth-change`) or workspace/ownership/isolation
 queries under `backend/app/persistence/` (`tenancy-change`).
@@ -61,6 +61,15 @@ invariant 8.
   only). Add allow **and** deny tests for any auth change (`backend/tests/test_auth.py`,
   `test_admin.py`).
 
+## Diagnostic logging
+
+Plain-text logs never receive raw line breaks from request-derived fields. The development-only
+magic-link fallback, CV review diagnostics, and application lifecycle logs strip CR/LF from email,
+URL, workspace, CV, application, and stage values before interpolation, preventing a caller from
+forging additional log entries. The renderer follows the same rule for upstream-fetch diagnostics:
+its format string is constant, every field removes line separators, and the session cookie is never
+logged. `test_auth.py` and `render/fetch.test.ts` pin those boundaries.
+
 ## Tenant isolation (invariant 8)
 
 Isolation is load-bearing even single-user: a member of workspace A must never read or write
diff --git a/docs/testing/strategy.md b/docs/testing/strategy.md
index 3c08144..2e57f44 100644
--- a/docs/testing/strategy.md
+++ b/docs/testing/strategy.md
@@ -45,6 +45,9 @@ silently ignored. A skipped check is a gap, not a pass. `--fail-on-gap` turns ga
 
 ## Test inventory
 
+- `scripts/check-doc-impact.test.mjs` — the doc-impact harness's pure rules and scratch-repository
+  end-to-end modes, including rejection of option-like refs before the argument-vector Git
+  subprocess runs. No shell command strings or network.
 - `backend/tests/test_schema_roundtrip.py` — camelCase round-trip + the provenance/sign-off gate.
 - `backend/tests/test_app.py` — health + the OpenAPI codegen-anchor guard (every model is emitted).
 - `frontend/src/slop/heuristics.test.ts` — the deterministic AI-language detector, anchored by id.
@@ -103,7 +106,8 @@ silently ignored. A skipped check is a gap, not a pass. `--fail-on-gap` turns ga
   magic callback logs an allowlisted address in and rejects others, **live revocation**
   (de-allowlist mid-session → next request exactly **403**), the **legacy-cookie back-compat**
   (a stored `role` key is ignored; `login_user` writes the legacy-compatible value for rollback
-  safety), and `/auth/entra/login` returns 503 while M365 is unconfigured. Runs offline (no Resend/Entra calls). Auth deps:
+  safety), the development magic-link log strips attacker-controlled CR/LF, and `/auth/entra/login`
+  returns 503 while M365 is unconfigured. Runs offline (no Resend/Entra calls). Auth deps:
   `authlib`, `itsdangerous`, `httpx` (`backend/pyproject.toml`).
 - `backend/tests/test_logging_config.py` — the `JsonFormatter` (`app/logging_config.py`): JSON output
   carries `level`/`message`/`logger`/`time`, exception records keep the rendered traceback, and
@@ -353,7 +357,11 @@ silently ignored. A skipped check is a gap, not a pass. `--fail-on-gap` turns ga
   / empty list → not ready.
 - `frontend/src/render/fetch.test.ts` — `getRenderBundle`: maps a 200 export to a `RenderInput` while
   forwarding the session cookie, maps a 409 to a blocked result carrying the blocking bullet ids, and
-  relays other non-200s. Offline with a stubbed `global.fetch`.
+  relays other non-200s. The unreachable-backend case returns 502 and proves every constant-format
+  diagnostic field is line-break-free. Offline with a stubbed `global.fetch`.
+- `frontend/src/lib/bootstrap-scripts.test.ts` — pins the theme and sidebar pre-paint scripts'
+  static storage-key literals to `THEME_KEY` / `RAIL_KEY`, so executable bootstrap source stays
+  interpolation-free without allowing the exported constants to drift.
 - `frontend/src/app/api/render/pdf/route.test.ts` — the PDF auth handoff: after the gate precheck the
   route **injects the caller's session cookie into the Playwright context**, and a 409 from the gate
   is relayed **without launching Chromium**. Playwright is mocked — offline.
diff --git a/frontend/src/lib/bootstrap-scripts.test.ts b/frontend/src/lib/bootstrap-scripts.test.ts
new file mode 100644
index 0000000..98b9278
--- /dev/null
+++ b/frontend/src/lib/bootstrap-scripts.test.ts
@@ -0,0 +1,13 @@
+import { describe, expect, it } from "vitest";
+import { RAIL_KEY, railInitScript } from "./sidebar";
+import { THEME_KEY, themeInitScript } from "./theme";
+
+describe("pre-paint bootstrap scripts", () => {
+  it("keeps the static theme storage key aligned with THEME_KEY", () => {
+    expect(themeInitScript).toContain(`localStorage.getItem('${THEME_KEY}')`);
+  });
+
+  it("keeps the static rail storage key aligned with RAIL_KEY", () => {
+    expect(railInitScript).toContain(`localStorage.getItem('${RAIL_KEY}')`);
+  });
+});
diff --git a/frontend/src/lib/sidebar.ts b/frontend/src/lib/sidebar.ts
index 44c7211..7c0dc06 100644
--- a/frontend/src/lib/sidebar.ts
+++ b/frontend/src/lib/sidebar.ts
@@ -7,6 +7,7 @@ export type RailState = "expanded" | "collapsed";
 
 export const RAIL_KEY = "contender-rail";
 
-export const railInitScript = `(function(){try{var r=localStorage.getItem(${JSON.stringify(
-  RAIL_KEY,
-)});document.documentElement.dataset.rail=(r==='collapsed')?'collapsed':'expanded';}catch(e){document.documentElement.dataset.rail='expanded';}})();`;
+// Static source by design: never interpolate runtime data into executable bootstrap code.
+// bootstrap-scripts.test.ts pins the duplicated storage-key literal to RAIL_KEY.
+export const railInitScript =
+  "(function(){try{var r=localStorage.getItem('contender-rail');document.documentElement.dataset.rail=(r==='collapsed')?'collapsed':'expanded';}catch(e){document.documentElement.dataset.rail='expanded';}})();";
diff --git a/frontend/src/lib/theme.ts b/frontend/src/lib/theme.ts
index 9e9cbc4..5143ff5 100644
--- a/frontend/src/lib/theme.ts
+++ b/frontend/src/lib/theme.ts
@@ -6,6 +6,7 @@ export type ThemeName = "dark" | "light";
 
 export const THEME_KEY = "contender-theme";
 
-export const themeInitScript = `(function(){try{var t=localStorage.getItem(${JSON.stringify(
-  THEME_KEY,
-)});if(t!=='light'&&t!=='dark'){t=window.matchMedia&&window.matchMedia('(prefers-color-scheme: light)').matches?'light':'dark';}document.documentElement.dataset.theme=t;}catch(e){document.documentElement.dataset.theme='dark';}})();`;
+// Static source by design: never interpolate runtime data into executable bootstrap code.
+// bootstrap-scripts.test.ts pins the duplicated storage-key literal to THEME_KEY.
+export const themeInitScript =
+  "(function(){try{var t=localStorage.getItem('contender-theme');if(t!=='light'&&t!=='dark'){t=window.matchMedia&&window.matchMedia('(prefers-color-scheme: light)').matches?'light':'dark';}document.documentElement.dataset.theme=t;}catch(e){document.documentElement.dataset.theme='dark';}})();";
diff --git a/frontend/src/render/fetch.test.ts b/frontend/src/render/fetch.test.ts
index f6807f2..e16d968 100644
--- a/frontend/src/render/fetch.test.ts
+++ b/frontend/src/render/fetch.test.ts
@@ -58,15 +58,22 @@ describe("getRenderBundle", () => {
   });
 
   it("returns a clean 502 (not a throw) when the backend is unreachable", async () => {
-    const cause = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED" });
+    const cause = Object.assign(new Error("connect\nECONNREFUSED"), { code: "ECONNREFUSED" });
     vi.stubGlobal(
       "fetch",
-      vi.fn(() => Promise.reject(Object.assign(new TypeError("fetch failed"), { cause }))),
+      vi.fn(() => Promise.reject(Object.assign(new TypeError("fetch\r\nfailed"), { cause }))),
     );
-    vi.spyOn(console, "error").mockImplementation(() => {});
+    const errorLog = vi.spyOn(console, "error").mockImplementation(() => {});
 
-    const result = await getRenderBundle("cv-123", "session=abc");
+    const result = await getRenderBundle("cv-\nforged", "session=abc", "ws-\r\nforged");
 
     expect(result).toEqual({ ok: false, status: 502, blockedBulletIds: [] });
+    expect(errorLog).toHaveBeenCalledOnce();
+    expect(errorLog.mock.calls[0]?.[0]).toBe(
+      "render fetch: upstream export failed workspace=%s cv=%s error=%s",
+    );
+    for (const field of errorLog.mock.calls[0] ?? []) {
+      expect(String(field)).not.toMatch(/[\r\n\u2028\u2029]/);
+    }
   });
 });
diff --git a/frontend/src/render/fetch.ts b/frontend/src/render/fetch.ts
index 2eb6595..cac172d 100644
--- a/frontend/src/render/fetch.ts
+++ b/frontend/src/render/fetch.ts
@@ -22,6 +22,12 @@ export type RenderFetch =
   | { ok: true; input: RenderInput }
   | { ok: false; status: number; blockedBulletIds: string[] };
 
+function logField(value: unknown): string {
+  return String(value)
+    .replace(/\n/g, "")
+    .replace(/[\r\u2028\u2029]/g, "");
+}
+
 /**
  * Fetch {master, cv} for `cvId`, forwarding `cookieHeader` for auth. `ok:false` carries the upstream
  * status (409 = needs sign-off, with the blocking bullet ids; 401/403/404 otherwise) so callers can
@@ -44,7 +50,12 @@ export async function getRenderBundle(
   } catch (err) {
     // Upstream connection failure (backend down/unreachable, ECONNREFUSED, DNS). Log the workspace +
     // cv id only — never the cookie. Don't let it bubble as a Next 500; relay a clean 502.
-    console.error(`render fetch: upstream export ${workspaceId}/${cvId} failed`, err);
+    console.error(
+      "render fetch: upstream export failed workspace=%s cv=%s error=%s",
+      logField(workspaceId),
+      logField(cvId),
+      logField(err),
+    );
     return { ok: false, status: 502, blockedBulletIds: [] };
   }
 
diff --git a/scripts/check-doc-impact.mjs b/scripts/check-doc-impact.mjs
index 7581c7e..666eb9b 100644
--- a/scripts/check-doc-impact.mjs
+++ b/scripts/check-doc-impact.mjs
@@ -37,7 +37,7 @@
  *
  * Node stdlib only. Favours false positives — acknowledge via the trailer.
  */
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import { readFileSync } from 'node:fs';
 import { join } from 'node:path';
 import { pathToFileURL } from 'node:url';
@@ -175,23 +175,27 @@ export function bucketsDiffer(before, after) {
 // Git plumbing + CLI
 // ---------------------------------------------------------------------------------------------
 
-let shCwd = process.cwd();
-const sh = (cmd) => {
+let repoCwd = process.cwd();
+const git = (...args) => {
   try {
-    return execSync(cmd, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], cwd: shCwd }).trim();
+    return execFileSync('git', args, {
+      encoding: 'utf8',
+      stdio: ['ignore', 'pipe', 'ignore'],
+      cwd: repoCwd,
+    }).trim();
   } catch {
     return '';
   }
 };
 
-const REF_RE = /^[\w./~^@{}-]+$/; // sanity-check user-supplied refs before shelling out
+const REF_RE = /^(?!-)[\w./~^@{}-]+$/; // reject malformed or option-like user-supplied refs
 
 function resolveRef(ref, flag) {
   if (!REF_RE.test(ref)) {
     console.error(`check-doc-impact: invalid ${flag} ref: ${ref}`);
     process.exit(1);
   }
-  const sha = sh(`git rev-parse --verify --quiet ${ref}^{commit}`);
+  const sha = git('rev-parse', '--verify', '--quiet', `${ref}^{commit}`);
   if (!sha) {
     console.error(`check-doc-impact: cannot resolve ${flag} ${ref}`);
     process.exit(1);
@@ -204,12 +208,12 @@ function depBuckets(path, source) {
   let blob;
   if (source && source.worktree) {
     try {
-      blob = readFileSync(join(shCwd, path), 'utf8');
+      blob = readFileSync(join(repoCwd, path), 'utf8');
     } catch {
       return null;
     }
   } else {
-    blob = sh(`git show ${source}:${path}`);
+    blob = git('show', `${source}:${path}`);
     if (!blob) return null; // absent / unreadable at this ref → unknown
   }
   try {
@@ -248,8 +252,10 @@ function main() {
   const againstArg = flagValue('--against');
   const headArg = flagValue('--head');
 
-  shCwd = sh('git rev-parse --show-toplevel') || process.cwd();
-  const head = headArg ? resolveRef(headArg, '--head') : sh('git rev-parse --verify --quiet HEAD');
+  repoCwd = git('rev-parse', '--show-toplevel') || process.cwd();
+  const head = headArg
+    ? resolveRef(headArg, '--head')
+    : git('rev-parse', '--verify', '--quiet', 'HEAD');
   if (!head) {
     console.log('check-doc-impact: no commits yet — skipping.');
     process.exit(0);
@@ -262,14 +268,14 @@ function main() {
   if (againstArg) {
     base = resolveRef(againstArg, '--against');
   } else {
-    const originMain = sh('git rev-parse --verify --quiet origin/main');
-    const mb = originMain ? sh(`git merge-base ${head} origin/main`) : '';
+    const originMain = git('rev-parse', '--verify', '--quiet', 'origin/main');
+    const mb = originMain ? git('merge-base', head, 'origin/main') : '';
     if (mb && mb !== head) {
       base = mb;
     } else if (worktree) {
       base = head; // on main / no upstream: lint the worktree changes only
     } else {
-      const prev = sh(`git rev-parse --verify --quiet ${head}~1`);
+      const prev = git('rev-parse', '--verify', '--quiet', `${head}~1`);
       if (!prev) {
         console.log('check-doc-impact: no prior commit to diff against — skipping (first commit).');
         process.exit(0);
@@ -280,7 +286,9 @@ function main() {
   }
 
   const committedDiff =
-    base === head ? [] : parseNameStatus(sh(`git diff --no-renames --name-status ${base} ${head}`));
+    base === head
+      ? []
+      : parseNameStatus(git('diff', '--no-renames', '--name-status', base, head, '--'));
   // --worktree: rules LIGHT from committed ∪ base→worktree ∪ untracked. The ack-exempt bucket is
   // the DIRTY SET ONLY (head→worktree ∪ untracked) — never base→worktree, which on a clean tree
   // reproduces the whole committed diff and would discard valid committed acks.
@@ -288,12 +296,15 @@ function main() {
   let untracked = [];
   let dirty = [];
   if (worktree) {
-    baseToWorktree = parseNameStatus(sh(`git diff --no-renames --name-status ${base}`));
-    untracked = sh('git ls-files --others --exclude-standard')
+    baseToWorktree = parseNameStatus(git('diff', '--no-renames', '--name-status', base, '--'));
+    untracked = git('ls-files', '--others', '--exclude-standard')
       .split('\n')
       .filter(Boolean)
       .map((p) => ({ status: 'A', path: p }));
-    dirty = [...parseNameStatus(sh(`git diff --no-renames --name-status ${head}`)), ...untracked];
+    dirty = [
+      ...parseNameStatus(git('diff', '--no-renames', '--name-status', head, '--')),
+      ...untracked,
+    ];
   }
   const lighting = [...committedDiff, ...baseToWorktree, ...untracked];
   const changedPaths = new Set(lighting.map((e) => e.path));
@@ -307,11 +318,18 @@ function main() {
 
   // Ack scan: non-merge commits in range, each judged on its OWN diff (computed lazily).
   const rangeCommits =
-    base === head ? [] : sh(`git log --no-merges --format=%H ${base}..${head}`).split('\n').filter(Boolean);
+    base === head
+      ? []
+      : git('log', '--no-merges', '--format=%H', `${base}..${head}`, '--')
+          .split('\n')
+          .filter(Boolean);
   const commitDiffCache = new Map();
   const commitDiff = (c) => {
     if (!commitDiffCache.has(c)) {
-      commitDiffCache.set(c, parseNameStatus(sh(`git diff --no-renames --name-status ${c}^ ${c}`)));
+      commitDiffCache.set(
+        c,
+        parseNameStatus(git('diff', '--no-renames', '--name-status', `${c}^`, c, '--')),
+      );
     }
     return commitDiffCache.get(c);
   };
@@ -320,7 +338,7 @@ function main() {
   // an ack riding a version-only bump must not validate a later structural change.
   const ackedOnLightingCommit = (rule) =>
     rangeCommits.some((c) => {
-      if (!hasValidAck(sh(`git log -1 --format=%B ${c}`), rule.id)) return false;
+      if (!hasValidAck(git('log', '-1', '--format=%B', c, '--'), rule.id)) return false;
       const diff = commitDiff(c);
       if (!ruleLit(rule, diff)) return false;
       if (rule.confirmLit) {
diff --git a/scripts/check-doc-impact.test.mjs b/scripts/check-doc-impact.test.mjs
index 04c03e5..f54d71e 100644
--- a/scripts/check-doc-impact.test.mjs
+++ b/scripts/check-doc-impact.test.mjs
@@ -262,6 +262,13 @@ test('--head isolates the linted tip (pre-push semantics)', (t) => {
   assert.equal(lint(work, '--fail-on-gap', '--head', shaA).status, 0, 'pushed tip A is clean');
 });
 
+test('option-like refs are rejected before invoking git', (t) => {
+  const { work } = makeRepos(t);
+  const r = lint(work, '--head', '--help');
+  assert.equal(r.status, 1, r.out);
+  assert.match(r.out, /invalid --head ref/);
+});
+
 test('a deleted required doc never satisfies its rule', (t) => {
   const { work } = makeRepos(t);
   write(work, 'backend/app/persistence/store.py', 'x = 2\n');