feat(ship-check): tool-definition reviewer saves its report to a temp file

[aliasunder]

Summary

• tool-definition-reviewer gains the Write tool and uses it for one file: the report path a dispatch names, when that path is absolute and under /tmp/, /private/tmp/, /var/folders/, or /private/var/folders/.
• With the report saved, the final message is the line Report file: <path>, then what the dispatch asks to return, then the Unfinished entries: and Status: lines. A long per-tool report stays out of the dispatcher's context.
• Shell writes stay banned, because the worktree sandbox refuses them. A path outside a temp directory, a runtime without Write, or a failed write still returns the full report inline under Report file not written: <path> — <reason>.
• The tool-definition-review skill states the same rule under "Report format", so a session running the skill inline follows it too.

Test plan

• Dispatch the reviewer with a scratchpad report path; confirm the file holds the full report and the final message is the short form.
• Dispatch with a repository path as the report file; confirm the inline fallback line and no file written.

🤖 Generated with Claude Code

[umm-actually]

Reject .. and symlink escapes in the report path check
Medium severity · security · high confidence

The temp-path guard is a lexical prefix check: any absolute path starting with /tmp/, /private/tmp/, /var/folders/, or /private/var/folders/ passes condition 2, so a path containing .. segments (e.g. /tmp/../../…) or a symlink under one of those roots resolves to a target outside the permitted roots. The agent would then call Write on a path the adjacent rule (NEVER call Write on any other path) intends to exclude. Verify containment on the resolved target, not the literal string.

Failure scenario: A dispatch names the report path /tmp/../../Users/alice/.ssh/config; the prefix check passes, the Write tool resolves the path outside /tmp, and the review report overwrites a file outside the allowed temp roots.

Suggested fix

Before writing, reject any named path containing a `..` segment, resolve the target's parent with realpath (or platform equivalent), and require the resolved parent to remain under one of the four listed temp roots; otherwise treat the path as failing condition 2 and return the report inline.

---

umm-actually · deepseek/deepseek-v4-flash-0731

[umm-actually]

Reject .. and symlink escapes in the report path check
Medium severity · security · high confidence

Same lexical-prefix containment gap as the agent copy: condition 2 accepts any absolute path starting with /tmp/, /private/tmp/, /var/folders/, or /private/var/folders/, so .. segments or a symlink under those roots escape to a file outside them while still passing the check. The standalone-skill session (which this block explicitly targets) has no stronger guard.

Failure scenario: A dispatch names /tmp/../etc/something or the equivalent via a symlinked directory; the prefix passes, the write lands outside the temp root, and the stated guarantee that the skill writes only its own report at a temp path is bypassed.

Suggested fix

Require the named path to contain no `..` segment, then resolve the target's parent and check the resolved path still starts with one of the four listed roots before calling Write.

---

umm-actually · deepseek/deepseek-v4-flash-0731

[umm-actually]

Scope the report-file fallback to dispatches that name a path
Low severity · correctness · high confidence

Condition 1 of the saving rule is itself a condition (the dispatch names a report file), so in the common case where the dispatch names no file, the fallback trigger — omitted: no; the literal text — applies: the agent must print Report file not written: <path> — … with no path to substitute. Every inline report on a dispatch without a report-file line would gain a malformed header (Report file not written: — the dispatch names no report file). The fallback should be scoped to dispatches that actually named a path.

Failure scenario: A user dispatches the reviewer without a report-file line: the agent follows the rule literally, prints Report file not written: <path> — … with an empty <path> above the title line, and the dispatcher expecting either a plain full report or a Report file: line receives a contradictory first line.

Suggested fix

Reword the fallback so it applies only when a path was named: "When the dispatch names a report file but condition 2 or 3 fails, or the `Write` call fails, put the line `Report file not written: <path> — <the failed condition or the error>` above the title line…" and state that a dispatch naming no file returns the full report inline with no prefix line.

---

umm-actually · deepseek/deepseek-v4-flash-0731

[umm-actually]

Scope the report-file fallback to dispatches that name a path
Low severity · correctness · high confidence

Same fallback-trigger incoherence as the agent copy: condition 1 is satisfying nothing; the analogous sentence fires when no file was named, leaving an empty <path> in the required Report file not written: <path> — <reason> line for every inline report of a dispatch without a report-file line.

Failure scenario: A session runs the skill inline without a report-file path in the briefing; the skill's own protocol instructs it to print a Report file not written: header that has no path to print, producing a malformed first line on the full report it then returns.

Suggested fix

Change the fallback trigger to "When the dispatch names a report file but condition 2 or 3 fails, or the `Write` call fails" and add that a dispatch naming no file returns the full report inline without the prefix line.

---

umm-actually · deepseek/deepseek-v4-flash-0731

[umm-actually]

umm-actually reviewed at b777a8f

4 new finding(s) posted (4 tracked finding(s) across all runs).

---

umm-actually · deepseek/deepseek-v4-flash-0731