docs: record launcher self-poisoning as an accepted risk (#417) - #8607
docs: record launcher self-poisoning as an accepted risk (#417)#8607iamwhatever wants to merge 1 commit into
Conversation
Closes #417. Problem: the engineering position on agent-launcher trust was already decided and written into the mechanism spec, but never closed out as a formal accepted risk. `grep -rn "CWE-778" docs/` returned zero hits and the "Known gaps" list in docs/architecture/security-deep-dive.md carried no launcher-substitution or self-poisoning entry, so a reader auditing the residual had nothing to find. What changed: one entry appended to the "Known gaps" list stating the residual, marking it ACCEPTED, and naming the tradeoff. It links to security.md's existing rationale rather than duplicating it. No behavior change. No code touched. The proposed code fix (a signed or root-owned launcher, install-source gating, the resolve-to-exec copy) is already recorded as rejected by design; this closes the paperwork gap, not the code question. CWE: the entry is filed under CWE-345 (Insufficient Verification of Data Authenticity), which is what the residual actually is -- the launcher's bytes are executed with no integrity or authenticity check. CWE-778 (Insufficient Logging) is retained as the tracker label from #371/#417 so the identifier people will grep for resolves, but it describes the SEL-audited-denial half of #371's defense, not the substitution risk.
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Records an already-decided residual in the document auditors actually read, matching the gaps list's shape and linking (not duplicating) the rationale — sound closure. The entry is internally consistent with the repo's existing threat model: it frames same-UID launcher writes as the same class as the [DESIGN-REVIEWED] 31cc284 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThis PR is a documentation-only addition to No findings. [OPUS-REVIEWED] 31cc284 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Problem / Motivation
The engineering position on agent-launcher trust was already decided and written
into the mechanism spec, but never closed out as a formal accepted risk.
docs/system-specs/modules/security.md(§ Kiro prerequisite setup boundary)records the reasoned rejection of exactly the mechanism #417 proposes: trust is
"the CLI runs, and it has a valid login" regardless of install source, owner, or
fixed path, because an owner / path / Developer-ID gate "would strand real
installs (toolbox, Homebrew, winget, a self-updated
/Applicationsbundle)" withno in-product recovery path. The sibling resolve-to-exec byte-binding copy is
recorded in the same place as deliberately removed ("Do NOT reintroduce it").
What was missing is the paperwork:
grep -rn "CWE-778" docs/returned zero hits on main — no accepted-riskrecord existed anywhere in the repo.
docs/architecture/security-deep-dive.mdcarriedno launcher-substitution or self-poisoning entry at all.
So a reader auditing the residual found a decision buried in a mechanism spec and
an empty gaps list, which is exactly the state #417's last triage pass describes.
Why it matters
The gaps list is the document a reviewer, auditor, or new maintainer reads to
learn what Kiro Crew deliberately does not defend. A residual that is decided
but not recorded reads as an oversight, and reopens the same code question every
time someone runs a scanner over the launcher path — which is how #417 arrived and
why it has been reopened for review three times since PR #371.
What changed (motivation → approach → change)
Goal: close the accepted-risk record without reopening the rejected code question.
Approach: one entry appended to the existing "Known gaps" list, following that
list's established shape (bold lead, then why the obvious fix is not already in
place). It links to
security.md's rationale rather than duplicating theprose, so the decision keeps one home.
Change — the entry states:
hash, ownership or install-source check, the only gate being
platform_compat.is_executable_file, so a same-UID agent can overwrite its ownlauncher and have those bytes executed on the next spawn;
self-updated bundle, plus Kiro CLI's own self-updater rewriting its bytes as the
user) over a same-UID integrity check;
which is outside this product's threat model and is not defended against
elsewhere either — the
~/.bashrcgap immediately above is the same class;run even a poisoned launcher inside Kiro Crew's own sandbox; only macOS
internal-sandbox delegation exec's it directly;
off, citing the
KIROCREW_PROVIDER_BIN_STRICTprecedent(
github_runner.py:validate_provider_executable), whose docstring records thatrequiring a root-owned copy made every stock package-manager install fail.
No behavior change. No code touched.
kiro_cli.pyand the launcher path areuntouched by design: #417's own history is that the proposed code fix was
descoped in PR #371 and the copy-then-exec variant deleted in PR #609. This PR
closes the paperwork gap, not the code question.
On the CWE (deliberate deviation from the issue's label)
The issue asks to "close CWE-778 as accepted-risk". CWE-778 is Insufficient
Logging, which is not what this residual is — nothing here is a logging gap, and
force-fitting it would leave the gaps list misclassifying its own entry.
The residual is that the launcher's bytes are executed with no verification of
their integrity or authenticity, which is CWE-345 (Insufficient Verification of
Data Authenticity). The entry is therefore filed under CWE-345 and also names
CWE-778 as the tracker label carried from #371/#417 — so
grep CWE-778nowresolves (the closure #417 asks for) while the classification is honest. CWE-778
most plausibly attached to the SEL-audited-denial half of PR #371's defense rather
than to substitution itself.
Flagging this explicitly for reviewer judgement: if CSE prefers the entry filed
under the tracker label alone, that is a one-word change.
Tests
N/A — documentation only, no code paths changed.
Gate run:
./scripts/docs-lint.sh→scanned 261 markdown files … All documentation checks passed(exit 0), pluspython3 scripts/docs_lint.py --test→
Self-test passed. This is the jobci.yml'sdocs-lintruns.Manual verification
688d67cff) thatgrep -rn "CWE-778" docs/returned zero hits before the change, and that the "Known gaps" list had no
launcher entry.
platform_compat.is_executable_file(src/kiro_crew/kiro_cli.py:245),validate_provider_executable(src/kiro_crew/github_runner.py:295),STRICT_PROVIDER_BIN_ENV = "KIROCREW_PROVIDER_BIN_STRICT"(
src/kiro_crew/github_runner.py:82).docs/system-specs/modules/security.md(the "valid login" sentence and the"Do NOT reintroduce it" line), and that the section title used in the link
(§ Kiro prerequisite setup boundary) is the enclosing subsection.
Related Issues
Closes #417.
Checklist
docs: …)