← back to Cli Printing Press

feat(skills): contain session-state.json outside DISCOVERY_DIR (#1425)

55b0f16bfb10fcd3ee948e9f87a73392909dc546 · 2026-05-14 23:33:34 -0700 · Trevin Chow

* feat(skills): contain session-state.json outside DISCOVERY_DIR

WU-5 of the amazon-orders retro (#955). Browser-sniff captures live
cookies/CSRF tokens into a session-state file; today the file lives at
$DISCOVERY_DIR/session-state.json and the Phase 5.5 archive block relies
on the operator remembering a manual `rm -f` before `cp -r
"$DISCOVERY_DIR"`. Missing that rm silently lands authentication
material into archived manuscripts, which future reprints can pick up.

Switch to containment-by-location:

- Run Initialization now defines $SESSION_DIR under
  ${TMPDIR:-/tmp}/printing-press/session/$RUN_ID and $SESSION_STATE_FILE
  underneath it, chmod 700.
- browser-sniff-capture.md write/read sites use $SESSION_STATE_FILE so
  agent-browser state save/restore never touches $DISCOVERY_DIR.
- Phase 5.5 archive cleanup now wipes $SESSION_DIR after archive; the
  legacy rm of $DISCOVERY_DIR/session-state.json stays as a no-op safety
  net for in-flight pre-isolation runs.
- secret-protection.md documents the new containment model.

Closes #961

* fix(skills): close TOCTOU window on $SESSION_DIR creation

Previously created $SESSION_DIR with `mkdir -p` and then ran a separate
`chmod 700`, leaving a brief window where the directory existed with
umask-derived perms (typically 0755). Switch to `(umask 077 && mkdir -p
"$SESSION_DIR")` so the directory lands at 0700 at creation.

Addresses Greptile P1/security finding on PR #1425.

* fix(skills): user-scope $SESSION_BASE so umask-077 mkdir does not block others

`mkdir -p` inherits umask for every intermediate directory it creates. The
previous `(umask 077 && mkdir -p "${TMPDIR:-/tmp}/printing-press/session/$RUN_ID")`
left `/tmp/printing-press/` itself at 0700 owned by the first operator on a
Linux host with shared /tmp, blocking concurrent operators/CI jobs from
traversing it.

Split into $SESSION_BASE="${TMPDIR:-/tmp}/printing-press-$(id -u)" and
$SESSION_DIR="$SESSION_BASE/session/$RUN_ID" so each operator gets their
own user-scoped subtree. macOS already provides a per-user $TMPDIR; the
$(id -u) suffix is a no-op there but keeps the path uniform across OSes.

Doc references in browser-sniff-capture.md and secret-protection.md updated
to reflect the new prefix.

Addresses Greptile P1/security finding on PR #1425.

Files touched

• M skills/printing-press/SKILL.md
• M skills/printing-press/references/browser-sniff-capture.md
• M skills/printing-press/references/secret-protection.md

Diff

commit 55b0f16bfb10fcd3ee948e9f87a73392909dc546
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Thu May 14 23:33:34 2026 -0700

    feat(skills): contain session-state.json outside DISCOVERY_DIR (#1425)
    
    * feat(skills): contain session-state.json outside DISCOVERY_DIR
    
    WU-5 of the amazon-orders retro (#955). Browser-sniff captures live
    cookies/CSRF tokens into a session-state file; today the file lives at
    $DISCOVERY_DIR/session-state.json and the Phase 5.5 archive block relies
    on the operator remembering a manual `rm -f` before `cp -r
    "$DISCOVERY_DIR"`. Missing that rm silently lands authentication
    material into archived manuscripts, which future reprints can pick up.
    
    Switch to containment-by-location:
    
    - Run Initialization now defines $SESSION_DIR under
      ${TMPDIR:-/tmp}/printing-press/session/$RUN_ID and $SESSION_STATE_FILE
      underneath it, chmod 700.
    - browser-sniff-capture.md write/read sites use $SESSION_STATE_FILE so
      agent-browser state save/restore never touches $DISCOVERY_DIR.
    - Phase 5.5 archive cleanup now wipes $SESSION_DIR after archive; the
      legacy rm of $DISCOVERY_DIR/session-state.json stays as a no-op safety
      net for in-flight pre-isolation runs.
    - secret-protection.md documents the new containment model.
    
    Closes #961
    
    * fix(skills): close TOCTOU window on $SESSION_DIR creation
    
    Previously created $SESSION_DIR with `mkdir -p` and then ran a separate
    `chmod 700`, leaving a brief window where the directory existed with
    umask-derived perms (typically 0755). Switch to `(umask 077 && mkdir -p
    "$SESSION_DIR")` so the directory lands at 0700 at creation.
    
    Addresses Greptile P1/security finding on PR #1425.
    
    * fix(skills): user-scope $SESSION_BASE so umask-077 mkdir does not block others
    
    `mkdir -p` inherits umask for every intermediate directory it creates. The
    previous `(umask 077 && mkdir -p "${TMPDIR:-/tmp}/printing-press/session/$RUN_ID")`
    left `/tmp/printing-press/` itself at 0700 owned by the first operator on a
    Linux host with shared /tmp, blocking concurrent operators/CI jobs from
    traversing it.
    
    Split into $SESSION_BASE="${TMPDIR:-/tmp}/printing-press-$(id -u)" and
    $SESSION_DIR="$SESSION_BASE/session/$RUN_ID" so each operator gets their
    own user-scoped subtree. macOS already provides a per-user $TMPDIR; the
    $(id -u) suffix is a no-op there but keeps the path uniform across OSes.
    
    Doc references in browser-sniff-capture.md and secret-protection.md updated
    to reflect the new prefix.
    
    Addresses Greptile P1/security finding on PR #1425.
---
 skills/printing-press/SKILL.md                     | 32 ++++++++++++++++++++--
 .../references/browser-sniff-capture.md            | 12 ++++----
 .../printing-press/references/secret-protection.md | 17 +++++++++---
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/skills/printing-press/SKILL.md b/skills/printing-press/SKILL.md
index e860807d..d6bb4b18 100644
--- a/skills/printing-press/SKILL.md
+++ b/skills/printing-press/SKILL.md
@@ -529,7 +529,28 @@ DISCOVERY_DIR="$API_RUN_DIR/discovery"
 CLI_WORK_DIR="$API_RUN_DIR/working/<api>-pp-cli"
 STAMP="$(date +%Y-%m-%d-%H%M%S)"
 
+# Session state (live cookies, CSRF tokens captured during authenticated
+# browser-sniff) lives OUTSIDE $API_RUN_DIR so the Phase 5.5 archive
+# `cp -r "$DISCOVERY_DIR"` cannot pick it up. Containment by location, not by
+# manual rm-before-archive.
+#
+# Base prefix is user-scoped (`printing-press-$(id -u)`) so that on a Linux
+# host with a shared /tmp, the umask-077 subshell below does not lock the
+# top-level `printing-press` directory to a single user. macOS already gives
+# us a per-user $TMPDIR; the $(id -u) suffix keeps semantics identical there.
+SESSION_BASE="${TMPDIR:-/tmp}/printing-press-$(id -u)"
+SESSION_DIR="$SESSION_BASE/session/$RUN_ID"
+SESSION_STATE_FILE="$SESSION_DIR/session-state.json"
+
 mkdir -p "$RESEARCH_DIR" "$PROOFS_DIR" "$PIPELINE_DIR" "$CLI_WORK_DIR"
+# Create $SESSION_DIR inside a subshell with a tight umask so it lands at 0700
+# at creation, not after a follow-up chmod. The two-step `mkdir; chmod` form
+# leaves a TOCTOU window where a concurrent process could open the directory
+# (and any session-state.json written into it) while perms are still
+# umask-derived (typically 0755 on Linux). The umask propagates to every
+# directory `mkdir -p` creates; the user-scoped $SESSION_BASE above is what
+# keeps that from blocking other users on the same host.
+(umask 077 && mkdir -p "$SESSION_DIR")
 STATE_FILE="$API_RUN_DIR/state.json"
 ```
 
@@ -2970,8 +2991,10 @@ cp -f "$API_RUN_DIR/research.json" "$PRESS_MANUSCRIPTS/$API_SLUG/$RUN_ID/researc
 cp -r "$PROOFS_DIR" "$PRESS_MANUSCRIPTS/$API_SLUG/$RUN_ID/proofs" 2>/dev/null || true
 
 # Archive discovery artifacts (browser-sniff captures, URL lists, traffic analysis, browser-sniff report).
-# Remove session state before archiving — contains authentication cookies/tokens.
-rm -f "$DISCOVERY_DIR/session-state.json"
+# Session state lives outside $DISCOVERY_DIR (see Run Initialization), so the
+# archive cannot pick it up. The legacy rm is a no-op safety net for an
+# in-flight $DISCOVERY_DIR carried over from a pre-isolation run.
+rm -f "$DISCOVERY_DIR/session-state.json" 2>/dev/null || true
 
 # Strip response bodies from HAR before archiving to control size.
 if [ -d "$DISCOVERY_DIR" ]; then
@@ -2982,6 +3005,11 @@ if [ -d "$DISCOVERY_DIR" ]; then
   done
   cp -r "$DISCOVERY_DIR" "$PRESS_MANUSCRIPTS/$API_SLUG/$RUN_ID/discovery" 2>/dev/null || true
 fi
+
+# Wipe live-auth scratch dir now that the run is archived. The directory lives
+# under ${TMPDIR:-/tmp}, so OS-level tmp reaping is the long-tail fallback, but
+# we clean explicitly so back-to-back runs do not accumulate session state.
+rm -rf "$SESSION_DIR" 2>/dev/null || true
 ```
 
 **MANDATORY: After archiving, you MUST proceed to Phase 6 below. Do not print a summary and stop. Do not treat archiving as the end of the run. The run ends when the user has been asked about next steps via the ship-path or hold-path menu.**
diff --git a/skills/printing-press/references/browser-sniff-capture.md b/skills/printing-press/references/browser-sniff-capture.md
index 8265bd4c..77f45b5a 100644
--- a/skills/printing-press/references/browser-sniff-capture.md
+++ b/skills/printing-press/references/browser-sniff-capture.md
@@ -223,14 +223,16 @@ For option 1 (save-then-restore):
 **IMPORTANT:** `--auto-connect`, `--state`, `--profile`, and `--headed` are daemon launch options in agent-browser. They only take effect when starting a new daemon. You MUST close the daemon between save and load.
 
 ```bash
-# Grab cookies from running Chrome
-agent-browser --auto-connect state save "$DISCOVERY_DIR/session-state.json" 2>&1
+# Grab cookies from running Chrome. $SESSION_STATE_FILE lives outside
+# $DISCOVERY_DIR (initialized in SKILL.md's "Run Initialization") so the
+# Phase 5.5 `cp -r "$DISCOVERY_DIR"` cannot pick it up.
+agent-browser --auto-connect state save "$SESSION_STATE_FILE" 2>&1
 
 # Close the auto-connect daemon so --state can start a fresh one
 agent-browser close 2>&1
 
 # Start a new headless daemon with the saved auth state
-agent-browser --state "$DISCOVERY_DIR/session-state.json" open <url>
+agent-browser --state "$SESSION_STATE_FILE" open <url>
 ```
 If auto-connect fails (no debug port), explain: "Chrome doesn't have remote debugging enabled. Quit Chrome and relaunch with `--remote-debugging-port=9222`, or pick option 2."
 
@@ -278,7 +280,7 @@ browser-use open <login-url> --headed --session "<api>-auth"
 Instruct the user: "A browser window is open. Please log in to `<site>`. Let me know when you're done."
 After login, save state:
 ```bash
-agent-browser state save "$DISCOVERY_DIR/session-state.json"
+agent-browser state save "$SESSION_STATE_FILE"
 ```
 Close the headed browser and restart headless with the saved state.
 
@@ -343,7 +345,7 @@ If the result is `SESSION_EXPIRED` (login link visible, no account link), the pr
 
 Do NOT silently proceed without auth when the session has expired. The authenticated surface is often the most valuable part of the API (order history, rewards, saved data).
 
-If cookies are verified, proceed to Steps 2a/2b capture flow with the authenticated session loaded. The session state file is stored at `$DISCOVERY_DIR/session-state.json`.
+If cookies are verified, proceed to Steps 2a/2b capture flow with the authenticated session loaded. The session state file is stored at `$SESSION_STATE_FILE` (under `${TMPDIR:-/tmp}/printing-press-$(id -u)/session/$RUN_ID/`, outside `$DISCOVERY_DIR`, so it cannot reach archived manuscripts).
 
 #### Step 2a.0: Direct-API-probe fallback (try before browser-use when WAF-protected)
 
diff --git a/skills/printing-press/references/secret-protection.md b/skills/printing-press/references/secret-protection.md
index 5aaa7fc2..8a771328 100644
--- a/skills/printing-press/references/secret-protection.md
+++ b/skills/printing-press/references/secret-protection.md
@@ -285,7 +285,16 @@ proceeds.
 
 ## Session state cleanup
 
-Session state files (`session-state.json`) contain browser cookies and auth tokens.
-The Phase 5.5 archive block removes them with `rm -f "$DISCOVERY_DIR/session-state.json"`.
-This removal is mandatory and must happen BEFORE the `cp -r "$DISCOVERY_DIR"` command.
-If the order is reversed, cookies leak into manuscripts.
+Session state files (`session-state.json`) contain live browser cookies and auth
+tokens captured during an authenticated browser-sniff run. The containment model
+is **by location**, not by archive-time cleanup: `SESSION_STATE_FILE` (set in
+SKILL.md's "Run Initialization") points at
+`${TMPDIR:-/tmp}/printing-press-$(id -u)/session/$RUN_ID/session-state.json`, outside
+`$DISCOVERY_DIR`. The Phase 5.5 archive `cp -r "$DISCOVERY_DIR"` therefore cannot
+pick it up, regardless of operator action. After the archive completes, the
+Phase 5.5 block also wipes `$SESSION_DIR` so back-to-back runs do not accumulate
+session state.
+
+A no-op `rm -f "$DISCOVERY_DIR/session-state.json"` remains in the archive
+block as a safety net for in-flight runs carried over from a pre-isolation
+skill version; it is not load-bearing for new runs.

← c5a51ead fix(cli): scope HOME/XDG_CONFIG_HOME for verify/dogfood subp  ·  back to Cli Printing Press  ·  fix(skills): canonicalize auth env var when slug-derived dif 2988814e →