Skip to content

docs: record launcher self-poisoning as an accepted risk (#417) - #8607

Open
iamwhatever wants to merge 1 commit into
mainfrom
docs/launcher-trust-signoff-417
Open

docs: record launcher self-poisoning as an accepted risk (#417)#8607
iamwhatever wants to merge 1 commit into
mainfrom
docs/launcher-trust-signoff-417

Conversation

@iamwhatever

Copy link
Copy Markdown
Collaborator

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 /Applications bundle)" with
no 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-risk
    record existed anywhere in the repo.
  • The "Known gaps" list in docs/architecture/security-deep-dive.md carried
    no 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 the
prose, so the decision keeps one home.

Change — the entry states:

  • the residual: the resolved launcher is executed in place with no signature,
    hash, ownership or install-source check, the only gate being
    platform_compat.is_executable_file, so a same-UID agent can overwrite its own
    launcher and have those bytes executed on the next spawn;
  • Status: ACCEPTED, explicitly;
  • the tradeoff by name: install-model compatibility (toolbox / Homebrew / winget /
    self-updated bundle, plus Kiro CLI's own self-updater rewriting its bytes as the
    user) over a same-UID integrity check;
  • why it is in-model: the attack presupposes local write access as the operator,
    which is outside this product's threat model and is not defended against
    elsewhere either — the ~/.bashrc gap immediately above is the same class;
  • the bounded blast radius: confined spawn paths (Linux namespace, macOS seatbelt)
    run even a poisoned launcher inside Kiro Crew's own sandbox; only macOS
    internal-sandbox delegation exec's it directly;
  • what a future enterprise posture would need, and that any such gate must default
    off, citing the KIROCREW_PROVIDER_BIN_STRICT precedent
    (github_runner.py:validate_provider_executable), whose docstring records that
    requiring a root-owned copy made every stock package-manager install fail.

No behavior change. No code touched. kiro_cli.py and the launcher path are
untouched 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-778 now
resolves (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.shscanned 261 markdown files … All documentation checks passed (exit 0), plus python3 scripts/docs_lint.py --test
Self-test passed. This is the job ci.yml's docs-lint runs.

Manual verification

  • Confirmed on this branch's base (688d67cff) that grep -rn "CWE-778" docs/
    returned zero hits before the change, and that the "Known gaps" list had no
    launcher entry.
  • Verified every symbol the entry cites actually exists on main:
    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).
  • Verified the quoted rationale is present verbatim in
    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.
  • Link target resolves: docs-lint enforces "every link resolves" and passed.

Related Issues

Closes #417.

Checklist

  • At most two commits (one), Conventional Commits title (docs: …)
  • Existing tests pass and new tests added for new functionality (N/A — docs)
  • Self-review completed; follows the gaps list's existing style
  • Documentation updated (this PR is the documentation update)
  • No secrets, credentials, or internal references in the diff

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.
@iamwhatever
iamwhatever requested a review from a team as a code owner September 5, 2026 00:42
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 31cc28468d4ed3460e0a048798617d353a9eaf59 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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 ~/.bashrc gap already on the list, names the bounded blast radius (confined spawn paths still sandbox a poisoned launcher, macOS internal-sandbox delegation is the exception), and cites the KIROCREW_PROVIDER_BIN_STRICT precedent for why a future gate must default off. The dual CWE filing (CWE-345 primary, CWE-778 as tracker label) keeps the classification honest while making the issue's grep closure resolve — the author flagged it for human judgment, which is the right handling. No behavior change, single-home rationale preserved.

[DESIGN-REVIEWED] 31cc284

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 31cc28468d4ed3460e0a048798617d353a9eaf59 — this comment is updated in place on each push.

Review details

This PR is a documentation-only addition to docs/architecture/security-deep-dive.md — prose recording an already-accepted security tradeoff. There is no executable code, schema, or runtime behavior change, so no candidate can meet the (a) concrete input / (b) call path / (c) observable wrong outcome bar. The discovery pass found no candidates, and nothing in the diff supplies a groundable defect.

No findings.

[OPUS-REVIEWED] 31cc284

Verdict parsed from the review's SHA-scoped output markers for commit 31cc28468d4ed3460e0a048798617d353a9eaf59.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 31cc28468d4ed3460e0a048798617d353a9eaf59: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 31cc28468d4ed3460e0a048798617d353a9eaf59 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 31cc284

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 31cc28468d4ed3460e0a048798617d353a9eaf59: <one-sentence reason>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

readiness: checking Automated validation is still running

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden agent-launcher trust against same-user self-poisoning (signed / root-owned launcher)

1 participant