Context
Filed from PR #1497 (fix for #1485 — grouping key + call/result previews in the detached observer's
distilled observations). Two independent automated reviews on that PR flagged a related but distinct
concern from #1485's own scope: the content now being persisted, not the schema shape.
Before #1497, a tool result appeared in the observations NDJSON only as an integer count
(tool_results: N); a tool call appeared only as a bare name. #1497 adds calls[].in (a bounded,
80-char preview of each tool call's input) and results[].out (an 80-char preview of each tool
result's output content), keyed by call id, so the headless analysis prompt can compute a
sequencing/batching/dependency finding instead of only ever dropping it.
Finding A (claude[bot] security review of PR #1497, IMPORTANT / PLAUSIBLE): results[].out can
now carry up to 80 characters of raw tool result content into the observations file — content that
can originate from external, third-party-controlled sources (files read via Read, WebFetch
responses, Bash output). That's more than enough for a prompt-injection payload. The observations
file is the untrusted data corpus fed to the headless claude -p analysis run; existing mitigations
(explicit trust-boundary language in the prompt, --tools Read restriction, shape-based _redact()
at the ledger write) are not new to this PR and don't fully close the gap — none of them prevent a
crafted injection from influencing what text the analysis returns, and that returned text is
written verbatim to the durable, persistent ledger. The say/human fields (160 chars) already
carried a version of this risk pre-PR, so the vector category isn't new, but this PR widens it: more
structured, more likely-verbatim external content, at a per-tool-result granularity that scales with
session length.
Finding B (chatgpt-codex-connector[bot] inline review of PR #1497, P2): when CLAUDE_PLUGIN_DATA
is unavailable on a multi-user Unix host, the documented launcher falls back to
/tmp/session-flow-observer; mkdir()/open("a") create that directory/file at the normal umask
(0755/0644). The new calls[].in previews can carry command/tool arguments — plausibly including
credentials passed as arguments — and collect-only or failed-analysis runs retain the observations
file, so another local user on that host could read data that, before this PR, was never copied into
the observations file at all.
Both findings were judged genuine but non-blocking for #1485/#1497 (that PR's own acceptance criteria
were about the grouping-key/dependency-preview capability, not the observer's file-permission or
injection-hardening posture) — deferred here per the post-green review pass's VALID-but-deferred rule.
Proposed work
- Create the observer's work directory and observations/status/lock/log files with owner-only
permissions (e.g. 0700/0600) rather than the current default-umask permissions, closing Finding
B regardless of CLAUDE_PLUGIN_DATA availability.
- Evaluate Finding A's suggested hardenings: a normalized/structural preview (e.g. only a first
path-like token) instead of raw content, and/or a redaction pass applied to calls[].in /
results[].out before they're written to the observations file (the same shape-based defense
already applied at the ledger write, moved one hop earlier) — note this only mitigates
secret/PII-shaped leakage, not free-text injection, so weigh whether it's worth doing alongside a
genuine assessment of whether the existing trust-boundary prompt language is sufficient on its own.
- Decide whether either change needs a
summarize_record() schema note (comment/docstring) so a
future reader understands why the preview is bounded/redacted the way it is.
Acceptance criteria
References
Metadata
| Field |
Value |
| Category |
session-flow / running-retro |
| Area |
session-flow |
| Ecosystem |
python |
This was generated by AI during work-loop execution.
Context
Filed from PR #1497 (fix for #1485 — grouping key + call/result previews in the detached observer's
distilled observations). Two independent automated reviews on that PR flagged a related but distinct
concern from #1485's own scope: the content now being persisted, not the schema shape.
Before #1497, a tool result appeared in the observations NDJSON only as an integer count
(
tool_results: N); a tool call appeared only as a bare name. #1497 addscalls[].in(a bounded,80-char preview of each tool call's input) and
results[].out(an 80-char preview of each toolresult's output content), keyed by call id, so the headless analysis prompt can compute a
sequencing/batching/dependency finding instead of only ever dropping it.
Finding A (
claude[bot]security review of PR #1497, IMPORTANT / PLAUSIBLE):results[].outcannow carry up to 80 characters of raw tool result content into the observations file — content that
can originate from external, third-party-controlled sources (files read via
Read,WebFetchresponses,
Bashoutput). That's more than enough for a prompt-injection payload. The observationsfile is the untrusted data corpus fed to the headless
claude -panalysis run; existing mitigations(explicit trust-boundary language in the prompt,
--tools Readrestriction, shape-based_redact()at the ledger write) are not new to this PR and don't fully close the gap — none of them prevent a
crafted injection from influencing what text the analysis returns, and that returned text is
written verbatim to the durable, persistent ledger. The
say/humanfields (160 chars) alreadycarried a version of this risk pre-PR, so the vector category isn't new, but this PR widens it: more
structured, more likely-verbatim external content, at a per-tool-result granularity that scales with
session length.
Finding B (
chatgpt-codex-connector[bot]inline review of PR #1497, P2): whenCLAUDE_PLUGIN_DATAis unavailable on a multi-user Unix host, the documented launcher falls back to
/tmp/session-flow-observer;mkdir()/open("a")create that directory/file at the normal umask(0755/0644). The new
calls[].inpreviews can carry command/tool arguments — plausibly includingcredentials passed as arguments — and collect-only or failed-analysis runs retain the observations
file, so another local user on that host could read data that, before this PR, was never copied into
the observations file at all.
Both findings were judged genuine but non-blocking for #1485/#1497 (that PR's own acceptance criteria
were about the grouping-key/dependency-preview capability, not the observer's file-permission or
injection-hardening posture) — deferred here per the post-green review pass's VALID-but-deferred rule.
Proposed work
permissions (e.g.
0700/0600) rather than the current default-umask permissions, closing FindingB regardless of
CLAUDE_PLUGIN_DATAavailability.path-like token) instead of raw content, and/or a redaction pass applied to
calls[].in/results[].outbefore they're written to the observations file (the same shape-based defensealready applied at the ledger write, moved one hop earlier) — note this only mitigates
secret/PII-shaped leakage, not free-text injection, so weigh whether it's worth doing alongside a
genuine assessment of whether the existing trust-boundary prompt language is sufficient on its own.
summarize_record()schema note (comment/docstring) so afuture reader understands why the preview is bounded/redacted the way it is.
Acceptance criteria
permissions on every supported platform, not just the
CLAUDE_PLUGIN_DATA-available path.preview-content hardening options.
References
calls[].in/results[].out; both findings are review comments on that PR)plugins/session-flow/skills/running-retro/scripts/observer.py:summarize_record()(previews),Observer.__init__(work-dir/file creation)Metadata
This was generated by AI during work-loop execution.