Keep thinker shellm config private and reliable

bin/shellm

@@ -11,6 +11,20 @@ set -euo pipefail
 # a parent shellm run) always wins over files — same semantics as
 # thinkers/_lib/common.sh:_load_env_defaults. Values are extracted by
 # sourcing the file in a subshell so quoting behaves like a plain `source`.
+# Preserve endpoint values from the original process separately: the two
+# endpoint names are aliases, so a process LLM_API_URL must also beat a
+# SHELLM_API_URL subsequently loaded from a file (and vice versa).
+_shellm_process_shellm_api_url_set=0
+_shellm_process_llm_api_url_set=0
+if [[ -n "${SHELLM_API_URL+x}" ]]; then
+    _shellm_process_shellm_api_url_set=1
+    _shellm_process_shellm_api_url="$SHELLM_API_URL"
+fi
+if [[ -n "${LLM_API_URL+x}" ]]; then
+    _shellm_process_llm_api_url_set=1
+    _shellm_process_llm_api_url="$LLM_API_URL"
+fi
+
 _shellm_load_env() {
     local envfile="$1"
     [[ -f "$envfile" ]] || return 1
@@ -70,7 +84,15 @@ SHELLM_MAX_ITERATIONS="${SHELLM_MAX_ITERATIONS:-}"
 SHELLM_MAX_CONSECUTIVE_FAILURES="${SHELLM_MAX_CONSECUTIVE_FAILURES:-10}"
 SHELLM_MAX_REPEAT_FAILURES="${SHELLM_MAX_REPEAT_FAILURES:-3}"
 SHELLM_TRUNCATE="${SHELLM_TRUNCATE:-2000}"
-SHELLM_API_URL="${SHELLM_API_URL:-}"
+if [[ "$_shellm_process_shellm_api_url_set" -eq 1 ]]; then
+    SHELLM_API_URL="$_shellm_process_shellm_api_url"
+elif [[ "$_shellm_process_llm_api_url_set" -eq 1 ]]; then
+    SHELLM_API_URL="$_shellm_process_llm_api_url"
+else
+    SHELLM_API_URL="${SHELLM_API_URL:-${LLM_API_URL:-}}"
+fi
+unset _shellm_process_shellm_api_url_set _shellm_process_shellm_api_url \
+    _shellm_process_llm_api_url_set _shellm_process_llm_api_url 2>/dev/null || true
 # Clear any inherited LLM_API_URL so llm uses per-provider defaults
 unset LLM_API_URL 2>/dev/null || true
 if [[ -n "$SHELLM_API_URL" ]]; then
@@ -2669,6 +2691,12 @@ exit \$__shellm_rc
         for _ev in "${_SHELLM_EXTRA_VARS[@]+"${_SHELLM_EXTRA_VARS[@]}"}"; do
             env_vars+=("$_ev")
         done
+        # Endpoint aliases are canonical shellm state, not ordinary extras.
+        # Keep the Docker-rewritten value last so a duplicate --var cannot
+        # restore a host-only URL inside generated code.
+        if [[ -n "$execution_api_url" ]]; then
+            env_vars+=(SHELLM_API_URL="$execution_api_url" LLM_API_URL="$execution_api_url")
+        fi
         if [[ "$SHELLM_DOCKER_ACCESS" == "broker" && -n "${_SHELLM_DOCKER_BROKER_DIR:-}" ]]; then
             env_vars+=(SHELLM_DOCKER_BROKER="$_SHELLM_DOCKER_BROKER_DIR")
             env_vars+=(SHELLM_DOCKER_TRANSPORT="$_SHELLM_DOCKER_BROKER_TRANSPORT")
@@ -2980,19 +3008,22 @@ exit \$__shellm_rc
 
 # The command line as recorded in the shellm-run trajectory row. Trajectories
 # get rendered into prompts, exported and shared, so `--var NAME=VALUE` values
-# whose NAME looks like a credential are masked. (Passing secrets as a bare
-# `--var NAME` keeps them off the command line altogether.)
+# whose NAME looks like a credential or endpoint are masked. (Passing private
+# configuration as a bare `--var NAME` keeps it off the process command line
+# altogether.)
+_shellm_private_var_name() {
+    local u
+    u=$(printf '%s' "$1" | tr '[:lower:]' '[:upper:]')
+    [[ "$u" =~ (KEY|TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?) \
+        || "$u" =~ (^|_)(URL|URI|ENDPOINT|DSN)(_|$) ]]
+}
+
 _redacted_cmdline() {
-    local out="shellm" prev="" a n nu
+    local out="shellm" prev="" a n
     for a in "$@"; do
         if [[ "$prev" == "--var" && "$a" == *=* ]]; then
             n="${a%%=*}"
-            # Uppercase via tr, not ${n^^}: the latter is a bash-4 expansion
-            # and macOS's /bin/bash is 3.2, where it is a fatal "bad
-            # substitution" that kills the whole run (every shellm run exited
-            # rc=1 with no output on macOS).
-            nu=$(printf '%s' "$n" | tr '[:lower:]' '[:upper:]')
-            if [[ "$nu" =~ (KEY|TOKEN|SECRET|PASSW|CREDENTIAL) ]]; then
+            if _shellm_private_var_name "$n"; then
                 a="$n=<redacted>"
             fi
         fi

tests/test_persona_bugreport.sh

@@ -40,7 +40,8 @@ check_not() { local label="$1"; shift; if "$@" >/dev/null 2>&1; then bad "$label
 export HOME="$WORK/home"
 export HEADLONG_HOME="$WORK/home/.headlong"
 export HEADLONG_APP_DIR="$WORK/app"
-mkdir -p "$HOME" "$HEADLONG_HOME/logs" "$HEADLONG_APP_DIR"
+export TMPDIR="$WORK/tmp"
+mkdir -p "$HOME" "$HEADLONG_HOME/logs" "$HEADLONG_APP_DIR" "$TMPDIR"
 ln -s "$REPO/bin" "$HEADLONG_APP_DIR/bin"
 ln -s "$REPO/tools" "$HEADLONG_APP_DIR/tools"
 ln -s "$REPO/thinkers" "$HEADLONG_APP_DIR/thinkers"
@@ -49,9 +50,12 @@ export PATH="$REPO/bin:$REPO/tools:$PATH"
 
 KEY="sk-or-v1-0123456789abcdef0123456789abcdef0123456789abcdef"
 PW="hunter2hunter2-very-secret"
+DSN="postgresql://bugreport-user:bugreport-pass@example.invalid/private-db"
+export BROKEN_SECRET=$'first-line\nsecond-line'
 cat > "$HEADLONG_HOME/.env" <<ENV
 OPENROUTER_API_KEY=$KEY
 SUPABASE_DB_PASSWORD=$PW
+DATABASE_DSN=$DSN
 SHELLM_MODEL=anthropic/claude-sonnet-4.5
 ENV
 printf 'headlong-web serving on http://127.0.0.1:8080\n' > "$HEADLONG_HOME/logs/web.log"
@@ -70,11 +74,14 @@ cat >> "$TJ/trajectory.jsonl" <<ROWS
 {"type":"shellm-run","cmd":"shellm --var SHELLM_MODEL=anthropic/claude-sonnet-4.5 --var OPENROUTER_API_KEY=$KEY think","ts":"2026-08-21T14:00:00Z"}
 {"type":"shellm-run","cmd":"shellm --var OPENROUTER_API_KEY think","ts":"2026-08-21T15:00:00Z"}
 {"type":"shellm-run","cmd":"shellm --var GH_TOKEN=ghp_OTHERTOKEN0123456789abcdefghijkl --var PG_PASSWORD=correcthorsebatterystaple --var SHELLM_ENV=local run","ts":"2026-08-21T15:30:00Z"}
+{"type":"shellm-run","cmd":"shellm --var DATABASE_DSN=$DSN --var CURL_OPTS=--retry=2 run","ts":"2026-08-21T15:31:00Z"}
+{"type":"shellm-run","cmd":"shellm --var SERVICE_APIKEY=svc_compact_0123456789abcdef --var APIKEY=api_compact_0123456789abcdef --var ACCESSTOKEN=tok_compact_0123456789abcdef --var PGPASSWORD=correcthorsebatterystaple run","ts":"2026-08-21T15:32:00Z"}
+{"type":"shellm-run","cmd":"shellm --var DB_PASSWORD=correct horse battery staple run","ts":"2026-08-21T15:33:00Z"}
 {"type":"thought","content":"the model is anthropic/claude-sonnet-4.5 and all is well","ts":"2026-08-21T15:00:01Z"}
 ROWS
 mkdir -p "$ID/run/logs" "$ID/memories" "$ID/workdir" "$TJ/blobs"
-printf 'env: OPENROUTER_API_KEY=%s\n' "$KEY" > "$ID/run/logs/monolith.log"
-printf -- '---\ntitle: db\n---\nthe db password is %s\n' "$PW" > "$ID/memories/db.md"
+printf 'env: OPENROUTER_API_KEY=%s\nmultiline: %s\n' "$KEY" "$BROKEN_SECRET" > "$ID/run/logs/monolith.log"
+printf -- '---\ntitle: db\n---\nthe db password is %s\nthe dsn is %s\n' "$PW" "$DSN" > "$ID/memories/db.md"
 printf 'scratch file\n' > "$ID/workdir/scratch.txt"
 head -c 2048 /dev/urandom > "$TJ/blobs/bin.dat"
 
@@ -115,21 +122,62 @@ check "binary blob carried intact"        cmp -s "$TJ/blobs/bin.dat" "$TOP/ident
 # 2. scrubbing
 check_not "API key value nowhere in bundle"   grep -rqF "$KEY" "$TOP"
 check_not "DB password nowhere in bundle"     grep -rqF "$PW" "$TOP"
+check_not "DSN literal nowhere in bundle"     grep -rqF "$DSN" "$TOP"
 check_not "no sk-... shaped string survives"  grep -rqE 'sk-[A-Za-z0-9_-]{8,}' "$TOP"
 check_not "other token value nowhere in bundle" grep -rqF 'ghp_OTHERTOKEN0123456789abcdefghijkl' "$TOP"
 check_not "other password nowhere in bundle"  grep -rqF 'correcthorsebatterystaple' "$TOP"
+check_not "compact API key values nowhere in bundle" grep -rqE '(svc|api)_compact_0123456789abcdef' "$TOP"
+check_not "compact access token nowhere in bundle" grep -rqF 'tok_compact_0123456789abcdef' "$TOP"
+check_not "multiline literal nowhere in bundle"     grep -rqF "$BROKEN_SECRET" "$TOP"
+check_not "spaced password suffix nowhere in bundle" grep -rqF 'horse battery staple' "$TOP"
 check "legacy --var KEY=value row masked, 4+4 hint kept" grep -q -- '--var OPENROUTER_API_KEY=<redacted sk-o...cdef> think' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "token not in .env still masked by pattern (hint kept)" grep -q -- '--var GH_TOKEN=<redacted ghp_...ijkl> ' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "password on argv masked whole"         grep -q -- '--var PG_PASSWORD=<redacted> --var SHELLM_ENV=local' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
+check "DSN on argv masked whole"              grep -q -- '--var DATABASE_DSN=<redacted> --var CURL_OPTS=--retry=2' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
+check "compact credential names are masked"   grep -q -- '--var SERVICE_APIKEY=<redacted svc_...cdef> --var APIKEY=<redacted api_...cdef> --var ACCESSTOKEN=<redacted tok_...cdef> --var PGPASSWORD=<redacted>' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
+check "CURL_OPTS is not a URL false positive" grep -q -- '--var CURL_OPTS=--retry=2 run' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check_not "no dangling hint tails"            grep -q -- '<redacted> [^ ]*>' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "bare --var KEY row untouched"          grep -q -- '--var OPENROUTER_API_KEY think' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "non-secret --var value kept"           grep -q -- '--var SHELLM_MODEL=anthropic/claude-sonnet-4.5' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "thinker log key scrubbed (hint kept)"  grep -q 'OPENROUTER_API_KEY=<redacted sk-o...cdef>$' "$TOP/identity/run/logs/monolith.log"
 check "memory password scrubbed"              grep -q 'password is <redacted>' "$TOP/identity/memories/db.md"
+check "memory DSN literal scrubbed"           grep -q 'dsn is <redacted>' "$TOP/identity/memories/db.md"
 check "plain thought text kept"               grep -q 'all is well' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl"
 check "source trajectory untouched"           grep -qF "$KEY" "$TJ/trajectory.jsonl"
 
-# 3. options
+# 3. helper failure is fail-closed; literals stay out of argv and temp files
+REAL_PYTHON=$(command -v python3)
+mkdir -p "$WORK/failbin" "$WORK/redact-state"
+cat > "$WORK/failbin/python3" <<EOF
+#!/usr/bin/env bash
+printf '%s\n' "\$@" >> "\$REDACT_STATE/python-args"
+[[ "\${REDACT_PYTHON_MODE:-}" != error ]] || exit 42
+exec '$REAL_PYTHON' "\$@"
+EOF
+chmod +x "$WORK/failbin/python3"
+
+rm -rf "$WORK/redact-state"; mkdir "$WORK/redact-state"
+if PATH="$WORK/failbin:$PATH" REDACT_STATE="$WORK/redact-state" REDACT_PYTHON_MODE=error \
+        persona alpha bugreport --out "$WORK/error-pass.tgz" >/dev/null 2>"$WORK/error-stderr"; then
+    bad "redactor failure aborts the bugreport"
+else
+    ok "redactor failure aborts the bugreport"
+fi
+check_not "redactor failure writes no archive" test -e "$WORK/error-pass.tgz"
+check "redactor failure is explained" grep -q 'secret scrubbing failed; no archive was written' "$WORK/error-stderr"
+check_not "failed helper output temp is removed" bash -c \
+    'find "$1" -name ".redact-output.*" -print -quit | grep -q .' _ "$WORK"
+if [[ -s "$WORK/redact-state/python-args" ]] \
+   && ! grep -qF "$KEY" "$WORK/redact-state/python-args" \
+   && ! grep -qF "$PW" "$WORK/redact-state/python-args" \
+   && ! grep -qF "$DSN" "$WORK/redact-state/python-args"; then
+    ok "redactor argv contains no private literals"
+else
+    bad "redactor argv contains no private literals"
+fi
+check_not "implementation writes no secret helper script" grep -q 'headlong-redact' "$REPO/tools/persona"
+
+# 4. options
 check "--include-workdir adds workdir" bash -c '
     persona alpha bugreport --out "$1" --include-workdir >/dev/null 2>&1 &&
     tar -tzf "$1" | grep -q "identity/workdir/scratch.txt"' _ "$WORK/b2.tgz"
@@ -139,6 +187,8 @@ check "--help exits 0 and mentions Usage" bash -c 'persona alpha bugreport --hel
 check_not "unknown option is an error"    persona alpha bugreport --bogus
 check "appears in persona --help"         bash -c 'persona alpha --help | grep -q bugreport'
 check_not "no stray staging dirs left"    bash -c 'ls -d "${TMPDIR:-/tmp}"/headlong-bugreport.* 2>/dev/null | grep -q .'
+check_not "no secret-bearing sed scripts exist" bash -c \
+    'find "$1" -name "headlong-redact.*" -print -quit | grep -q .' _ "$WORK"
 
 printf '\n%d passed, %d failed\n' "$pass" "$fail"
 [[ "$fail" -eq 0 ]]

tests/test_thinker_env_fallback.sh

@@ -125,6 +125,95 @@ case "$out" in
     *) bad "the key is forwarded to the nested shellm" "no bare --var for it" ;;
 esac
 
+# A shell-local provider key must be exported before flags are assembled in a
+# process substitution. Exercise a real nested shellm: without the parent
+# export, its bare --var parser fails before generated code can see the key.
+out=$(
+    H=$(mktemp -d); trap 'rm -rf "$H"' EXIT; export HOME="$H"
+    unset ANTHROPIC_API_KEY OPENAI_API_KEY GEMINI_API_KEY OPENROUTER_API_KEY \
+          OPENCODE_API_KEY LLM_API_KEY HEADLONG_HOME SHELLM_HOME
+    mkdir -p "$H/id/memories" "$H/id/skills" "$H/id/kernel" "$H/id/trajectories" "$H/wd" "$H/bin"
+    printf 'name=probe\n' > "$H/id/info.txt"
+    cat > "$H/bin/llm" <<'EOF'
+#!/usr/bin/env bash
+printf '%s\n' '```bash' 'if [[ -n "${OPENROUTER_API_KEY:-}" ]]; then FINAL=provider-key-present; else FINAL=provider-key-missing; fi' '```'
+EOF
+    chmod +x "$H/bin/llm"
+    export PATH="$H/bin:$REPO/bin:$REPO/tools:$PATH"
+    export IDENTITY_DIR="$H/id" TRAJ_DIR="$H/id/trajectories" TRAJ_ID=t1 MEM_DIR="$H/id/memories"
+    export SHELLM_MODEL=test-model SHELLM_THINKER_ENV=local
+    OPENROUTER_API_KEY=synthetic-provider-key
+    cd "$H/wd" || exit 1
+    # shellcheck disable=SC1090  # the library under test
+    source "$REPO/thinkers/_lib/common.sh"
+    _require_env >/dev/null 2>&1
+    _export_provider_keys
+    flags=()
+    while IFS= read -r flag; do
+        [[ -n "$flag" ]] && flags+=("$flag")
+    done < <(_build_shellm_flags "$IDENTITY_DIR" "$H/wd")
+    nested_out=$("$REPO/bin/shellm" "${flags[@]}" --max-iterations 1 key-probe 2>/dev/null)
+    printf '%s\n' "$nested_out"
+    row=$(grep -rh '"type":"shellm-run"' "$H/id/trajectories" 2>/dev/null | tail -1)
+    if [[ "$row" == *"--var OPENROUTER_API_KEY "* && "$row" != *"synthetic-provider-key"* ]]; then
+        printf 'provider-key-command-safe\n'
+    fi
+)
+if [[ "$out" == *provider-key-present* && "$out" == *provider-key-command-safe* ]]; then
+    ok "an unexported provider key reaches an actual nested shellm"
+else
+    bad "an unexported provider key reaches an actual nested shellm" "got $out"
+fi
+
+# Endpoints are inherited rather than repeated as --var, because shellm owns
+# their Docker rewrite. Skill-declared values still use bare --var NAME, and an
+# existing-but-unexported skill value must be exported in the parent before
+# flag assembly runs in process substitution.
+out=$(
+    H=$(mktemp -d); trap 'rm -rf "$H"' EXIT; export HOME="$H"
+    unset HEADLONG_HOME SHELLM_HOME
+    mkdir -p "$H/id/memories" "$H/id/skills/probe" "$H/id/kernel" "$H/id/trajectories" "$H/wd"
+    printf 'name=probe\n' > "$H/id/info.txt"
+    cat > "$H/id/skills/probe/SKILL.md" <<'EOF'
+---
+name: probe
+description: Test fixture.
+metadata:
+  shelllm:
+    requires:
+      env: ["PROBE_SERVICE_CONFIG", "LLM_API_URL", "SHELLM_API_URL"]
+---
+EOF
+    export IDENTITY_DIR="$H/id" TRAJ_DIR="$H/id/trajectories" TRAJ_ID=t1 MEM_DIR="$H/id/memories"
+    export LLM_API_URL="https://example.invalid/v1/responses"
+    unset SHELLM_API_URL
+    # Intentionally unexported: _export_skill_vars must promote it for --var NAME.
+    # shellcheck disable=SC2034
+    PROBE_SERVICE_CONFIG="private-config-canary"
+    cd "$H/wd" || exit 1
+    # shellcheck disable=SC1090  # the library under test
+    source "$REPO/thinkers/_lib/common.sh"
+    _require_env >/dev/null 2>&1
+    _export_skill_vars "$IDENTITY_DIR"
+    _build_shellm_flags "$IDENTITY_DIR" 2>/dev/null | tr '\n' ' '
+    if bash -c '[[ -n "${LLM_API_URL:-}" && -n "${PROBE_SERVICE_CONFIG:-}" ]]'; then
+        printf 'parent-export-ok '
+    fi
+)
+case "$out" in
+    *"example.invalid"*|*"private-config-canary"*) bad "private config values stay out of thinker shellm flags" ;;
+    *)
+        if [[ "$out" != *"--var SHELLM_API_URL "* \
+           && "$out" != *"--var LLM_API_URL "* \
+           && "$out" == *"--var PROBE_SERVICE_CONFIG "* \
+           && "$out" == *"parent-export-ok "* ]]; then
+            ok "endpoint is inherited and an unexported skill var is parent-exported"
+        else
+            bad "endpoint is inherited and an unexported skill var is parent-exported" "unexpected flags or missing parent export"
+        fi
+        ;;
+esac
+
 echo
 echo "$pass passed, $fail failed"
 [[ $fail -eq 0 ]]