Security hardening, CLI bug fixes, and SARIF report output

[DevamShah]

Drive-by contribution from a long-time Shannon user. I'm aware of the
"external PRs not accepted at this time" notice in the README — happy
to close this and refile via Issues / private channel if preferred,
or split into smaller pieces. Three orthogonal commits, each
independently revertible.

Summary

Three logically distinct commits, one PR for review convenience:

1. security: hardening — prompt-injection defences in
   prompt-manager.ts, UID/GID validation in entrypoint.sh, drop
   chmod 777 to chmod 770 on container temp dirs.
2. fix(cli): — URL try/catch + scheme allowlist on start,
   distinguish ENOENT from real I/O errors in the session-poll loop,
   locale-aware splash with ASCII fallback for terminals that don't
   render Unicode block art.
3. feat: SARIF 2.1.0 report output — opt-in --report-format sarif
   flag wires a new SarifReportOutputProvider through the existing
   ReportOutputProvider DI seam. Default behaviour (md only) is
   byte-for-byte unchanged.

Why bundled

These were found while reading the codebase to evaluate Shannon for
internal use. Each commit stands alone — squash, cherry-pick, or close
any one without affecting the others.

Commit 1 — security

ID	What	Where
S-1	Prompt injection via config.description	apps/worker/src/services/prompt-manager.ts
S-2	Credential injection (username / password / TOTP secret) into prompts	same
S-3	config.avoid / config.focus rule descriptions injected raw	same
S-4	SHANNON_HOST_UID / SHANNON_HOST_GID consumed by groupadd / useradd without numeric or range validation	entrypoint.sh
S-5	chmod 777 on /app, /tmp/.cache, /tmp/.config, /tmp/.npm	Dockerfile

Threat model. Anyone who can write a Shannon config (a CI secret
leak, a compromised target repo) can today embed {{AUTH_CONTEXT}}
or @include(/etc/passwd) in a description and have the agent treat
it as orchestrator-level instruction. After this change those payloads
are inert text — the new `sanitizePromptValue()` breaks {{...}}
placeholder syntax and @include(...) directives. Newlines are
preserved. Applied uniformly to every user-controlled interpolation
site.

`entrypoint.sh` now rejects non-numeric / out-of-range / zero values
for `SHANNON_HOST_UID` and `SHANNON_HOST_GID` with a clear error,
preventing a malicious env from mapping the pentest user to root or
feeding crafted input into a system command.

`chmod 770` is sufficient: only the pentest user (or a UID remapped
into the pentest group) ever runs in the container — world-write adds
blast radius without functional benefit.

Commit 2 — CLI bug fixes

• `new URL(args.url)` was called twice deep in setup with no error
  handling; a malformed input crashed mid-setup with a raw `TypeError`.
  Now wrapped in try/catch with an explicit `http` / `https` scheme
  allowlist (the worker assumes web semantics).
• The `session.json` polling loop's bare `catch` swallowed
  `EACCES` / `EIO` / `ENOTDIR` alike, so a permissions issue
  manifested as an indefinite "Waiting for workflow to start...".
  Now distinguishes `ENOENT` (steady-state) and `SyntaxError`
  (worker mid-write) from real I/O errors.
• `splash.ts` falls back to a plain-ASCII variant when the terminal
  doesn't advertise UTF-8. Detection uses `LANG` / `LC_ALL` /
  `LC_CTYPE` plus the well-known `WT_SESSION` and
  `TERM_PROGRAM=vscode` signals. The Unicode visual is preserved on
  every modern terminal and only degrades on raw cmd.exe / locale-less
  SSH / some CI log streams.

Commit 3 — SARIF report output

`shannon start ... --report-format sarif` writes
`/.shannon/deliverables/comprehensive_security_assessment_report.sarif`
alongside the markdown report. The default (`md`) is unchanged.

Wiring goes:
`CLI flag → SHANNON_REPORT_FORMAT env → worker.ts:configureReportOutputProvider() → setContainerFactory(SarifReportOutputProvider)`

The provider plugs into the existing `ReportOutputProvider` interface
that's already invoked from `generateReportOutputActivity` —
zero changes to the activity / workflow layer.

Tool driver advertises five rules tagged with their CWE IDs and
the canonical OWASP help URI:
`shannon.injection` (CWE-74), `shannon.xss` (CWE-79),
`shannon.auth` (CWE-287), `shannon.ssrf` (CWE-918),
`shannon.authz` (CWE-285). Each non-empty
`*_exploitation_evidence.md` produces one SARIF `result` with the
evidence body as the message (truncated at 16 KiB to stay under
GitHub's per-result cap).

Out of scope (explicitly v0.1). Per-finding line/column locations
inside source files. The agents don't currently emit structured
per-finding metadata, so the artefact location is the deliverable file
itself. The envelope and consumer wiring shipped here unblock that
follow-up — the result mapping is then a one-function change.

Test plan

• `pnpm test` — 21/21 vitest cases green across three suites:
  • `prompt-manager.test.ts` (11 cases) pins the
    `sanitizePromptValue` and URL validation contracts
  • `uid-gid-validation.test.ts` (5 cases) pins the regex+range
    contract that `entrypoint.sh` enforces, so the bash and TS
    sides can never silently drift
  • `sarif-output-provider.test.ts` (5 cases) covers SARIF envelope
    shape, one-result-per-evidence, empty/missing handling,
    oversized truncation, and the output path
• `pnpm check` — `tsc --noEmit` clean for both `@keygraph/shannon` and `@shannon/worker`
• `pnpm build` — turbo build succeeds; `apps/cli/dist/index.mjs`
  bundle 53.21 kB / 14.48 kB gzipped (no growth from main)
• Manual: `node apps/cli/dist/index.mjs help` shows the new flag;
  `LANG=C node apps/cli/dist/index.mjs info` renders the ASCII
  fallback splash; `pnpm check` / `pnpm build` / `pnpm test` all
  cached on second run
• Pre-existing biome issues on `main` (import-organize warnings in
  `workflows.ts` etc.) are not touched. All files added or modified
  in this PR pass `biome check` cleanly.

Things I deliberately did not do

• No `exec $*` quoting fix in `entrypoint.sh`. The args come
  from Docker `CMD`, not user input — the practical injection risk is
  near zero, and the fix is fiddly enough that I'd rather not bundle
  it.
• No `--max-cost` cap or `--dry-run` mode. Both were on my
  shortlist but each needs cost-tracker / fs-write-audit infrastructure
  I don't yet have a clean read on. Happy to follow up with separate
  PRs if there's interest.
• No SARIF result locations inside source files. Out of scope until
  agents emit structured findings (see Commit 3 description).

---

Built against `main @ 79caada` (post-v1.1.0). Author: Devam Shah —
no commercial affiliation, contributing personally.

[DevamShah]

@ezl-keygraph — this sat for four months at +1677/-40 because I bundled three unrelated things into one PR. That was my mistake, so I've cut it down rather than ask you to wade through it.

Dropped, and why:

• SARIF output — you shipped it in feat: multi-provider model support, SARIF output, and exploit-mode fixes #402 and made it the default for exploit runs in fix(report): emit SARIF by default for exploit runs #431. Fully superseded; deleted.
• Prompt-interpolation sanitiser — replaceLiteral, plus running processIncludes before interpolateVariables, already closes most of what I was guarding against. Not worth the diff.
• ASCII splash fallback — splash.ts:3 now states the Unicode art is deliberately always kept. Your call; dropped.
• vitest + a turbo test task — the repo has no test framework today, and adopting one is a decision for you to make, not something to smuggle in under a security PR.

What's left is rebased onto main (a650708) at +26/-6 across two files: reject non-numeric / 0 / out-of-range SHANNON_HOST_UID and SHANNON_HOST_GID before they reach groupadd/useradd, and drop chmod 777 → 770 on /app, /tmp/.cache, /tmp/.config, /tmp/.npm.

Worth flagging, because it's the non-obvious half: the 777 is load-bearing today. entrypoint.sh re-chowns /app/sessions, /app/workspaces, /tmp/.claude and /tmp/.pi after a uid remap, but never the three /tmp cache dirs — they stay writable by the remapped uid only because they're world-writable. Tightening the mode alone would break every SHANNON_HOST_UID run, so the commit extends that chown as well.

Verified: docker build --check . → Check complete, no warnings found. Validation exercised directly against entrypoint.sh — 0, -1, abc, 1001; rm -rf /, 3000000, and an empty GID each exit 1 with the error; 1000/1000 passes and proceeds to groupadd; unset skips the remap branch entirely, unchanged. I have not built the full image or run a container with a remapped uid end-to-end.

Two things I pulled out and deliberately did not open PRs for — say the word if either is wanted:

• apps/cli/src/commands/start.ts:235 — the session.json poll swallows every error in a bare catch {}, so an EACCES or EIO surfaces as 120s of "Waiting for the scan to start" followed by a timeout, with no diagnostic.
• apps/cli/src/index.ts:138 — new URL(url) validates parseability but not scheme, so file:// and javascript: pass.

The title and description still describe the old three-part scope; treat this comment as the description until I correct them.

[DevamShah]

I re-checked this against main at 25b90b0 — it still rebases clean, same two files, +26/-6, and nothing upstream has overtaken it. The Dockerfile still has chmod 777 on /app and the three /tmp cache dirs at 136-139, and entrypoint.sh still passes SHANNON_HOST_UID and GID straight into groupadd and useradd with no validation.

The one thing worth spelling out is why it's two files and not two separate one-line patches. That 777 is the only reason /tmp/.cache, /tmp/.config and /tmp/.npm stay writable once the uid is remapped, because the chown on line 15 only ever covered sessions, workspaces, .claude and .pi. Tighten the mode without extending that chown and every SHANNON_HOST_UID run breaks, so the two halves have to land together.

I know the README says you're not taking external code, and I'm not going to argue the policy. If that's still firm, just say so and I'll close this and refile the same two hunks as an issue — I'd rather you had the fix than have this sitting on your queue.