diff --git a/bin/shellm b/bin/shellm index 90f6c9b..570ae90 100755 --- a/bin/shellm +++ b/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=" fi fi diff --git a/tests/test_persona_bugreport.sh b/tests/test_persona_bugreport.sh index dbc3cae..77765d6 100755 --- a/tests/test_persona_bugreport.sh +++ b/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" < "$HEADLONG_HOME/logs/web.log" @@ -70,11 +74,14 @@ cat >> "$TJ/trajectory.jsonl" < "$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= think' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl" check "token not in .env still masked by pattern (hint kept)" grep -q -- '--var GH_TOKEN= ' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl" check "password on argv masked whole" grep -q -- '--var PG_PASSWORD= --var SHELLM_ENV=local' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl" +check "DSN on argv masked whole" grep -q -- '--var DATABASE_DSN= --var CURL_OPTS=--retry=2' "$TOP/identity/trajectories/$(basename "$TJ")/trajectory.jsonl" +check "compact credential names are masked" grep -q -- '--var SERVICE_APIKEY= --var APIKEY= --var ACCESSTOKEN= --var PGPASSWORD=' "$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 -- ' [^ ]*>' "$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=$' "$TOP/identity/run/logs/monolith.log" check "memory password scrubbed" grep -q 'password is ' "$TOP/identity/memories/db.md" +check "memory DSN literal scrubbed" grep -q 'dsn is ' "$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" <> "\$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 ]] diff --git a/tests/test_thinker_env_fallback.sh b/tests/test_thinker_env_fallback.sh index 3705e8e..45758f0 100755 --- a/tests/test_thinker_env_fallback.sh +++ b/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 ]] diff --git a/tests/test_var_secrets.sh b/tests/test_var_secrets.sh index c9dc754..9236f60 100755 --- a/tests/test_var_secrets.sh +++ b/tests/test_var_secrets.sh @@ -9,8 +9,8 @@ # 2. While the generated code runs, the secret's value appears in no # process's argv (`ps`): not shellm's, not `env`'s, not bash's. # 3. The shellm-run trajectory row records the command with legacy -# `--var SOME_KEY=value` values masked, and the literal value is -# nowhere under the state home. +# credential and endpoint `--var NAME=value` values masked, and the +# literal values are nowhere under the state home. # # `llm` is stubbed (canned fenced blocks, no network), same pattern as # tests/test_inactivity_beacon.sh. Local execution is pinned: shellm would @@ -64,6 +64,15 @@ run_shellm() { # (this script's own text included) unless something actually leaks them. SECRET="sk-test-secret-$$-$RANDOM$RANDOM" LEGACY="sk-legacy-literal-$$-$RANDOM$RANDOM" +PRIVATE_URL="https://example.invalid/private/$$?opaque=$RANDOM" +PRIVATE_DSN="postgresql://user:password@example.invalid/private_$$" +INHERITED_URL="http://127.0.0.1:9/private/$RANDOM" +FILE_URL="http://file.example.invalid/v1/$RANDOM" +COMPACT_APIKEY="api-compact-secret-$$-$RANDOM$RANDOM" +COMPACT_BARE_KEY="bare-key-compact-secret-$$-$RANDOM$RANDOM" +COMPACT_TOKEN="token-compact-secret-$$-$RANDOM$RANDOM" +COMPACT_PASSWORD="password-compact-secret-$$-$RANDOM$RANDOM" +CURL_VALUE="--retry=2" # --- 1. bare --var with the variable unset is an error ------------------------ unset SECRET_PROBE @@ -76,15 +85,34 @@ fi # --- 2. forwarded value reaches the code; nothing carries it in argv ---------- export SECRET_PROBE="$SECRET" -fence "printf 'got=%s plain=%s\\n' \"\$SECRET_PROBE\" \"\$PLAIN\" > '$WORK/probe.txt' +unset SHELLM_API_URL +export LLM_API_URL="$INHERITED_URL" +printf 'SHELLM_API_URL=%s\n' "$FILE_URL" > "$WORK/wd/.env" +fence "printf 'got=%s plain=%s api=%s shellm=%s\\n' \"\$SECRET_PROBE\" \"\$PLAIN\" \"\$LLM_API_URL\" \"\$SHELLM_API_URL\" > '$WORK/probe.txt' ps -axo args= > '$WORK/ps.txt' 2>/dev/null || ps -eo args= > '$WORK/ps.txt'" > "$WORK/script/1" fence 'FINAL=done' > "$WORK/script/last" -run_shellm --var SECRET_PROBE --var PLAIN=1 --var "OPENROUTER_API_KEY=$LEGACY" "task" -if grep -qx "got=$SECRET plain=1" "$WORK/probe.txt" 2>/dev/null; then +run_shellm --var SECRET_PROBE --var PLAIN=1 --var "OPENROUTER_API_KEY=$LEGACY" \ + --var "SERVICE_URL=$PRIVATE_URL" --var "DATABASE_DSN=$PRIVATE_DSN" \ + --var "SHELLM_API_URL=$FILE_URL" \ + --var "SERVICE_APIKEY=$COMPACT_APIKEY" --var "APIKEY=$COMPACT_BARE_KEY" \ + --var "ACCESSTOKEN=$COMPACT_TOKEN" \ + --var "PGPASSWORD=$COMPACT_PASSWORD" \ + --var "CURL_OPTS=$CURL_VALUE" "task" +if grep -q "^got=$SECRET plain=1 " "$WORK/probe.txt" 2>/dev/null; then ok "bare --var NAME forwards the value into the generated code" else bad "bare --var NAME forwards the value into the generated code" "probe: $(cat "$WORK/probe.txt" 2>/dev/null) err: $(tail -2 "$WORK/err")" fi +if grep -q " api=$INHERITED_URL shellm=$INHERITED_URL$" "$WORK/probe.txt" 2>/dev/null; then + ok "process LLM_API_URL wins over file and duplicate extra endpoint aliases" +else + bad "process LLM_API_URL wins over file and duplicate extra endpoint aliases" "probe: $(cat "$WORK/probe.txt" 2>/dev/null)" +fi +if [[ -z "${SHELLM_API_URL+x}" ]]; then + ok "the shellm run started with LLM_API_URL only" +else + bad "the shellm run started with LLM_API_URL only" +fi if [[ -s "$WORK/ps.txt" ]]; then if ! grep -qF "$SECRET" "$WORK/ps.txt"; then ok "forwarded secret is in no process's argv while the code runs" @@ -109,15 +137,33 @@ fi # --- 3. recorded command is redacted; literal value nowhere in state ---------- row=$(grep -rh '"type":"shellm-run"' "$HEADLONG_HOME" 2>/dev/null | tail -1) -if [[ -n "$row" ]] && grep -qF 'OPENROUTER_API_KEY=' <<<"$row" && ! grep -qF "$LEGACY" <<<"$row"; then - ok "shellm-run row masks credential-looking --var values" +if [[ -n "$row" ]] && grep -qF 'OPENROUTER_API_KEY=' <<<"$row" \ + && grep -qF 'SERVICE_URL=' <<<"$row" \ + && grep -qF 'DATABASE_DSN=' <<<"$row" \ + && grep -qF 'SERVICE_APIKEY=' <<<"$row" \ + && grep -qF 'APIKEY=' <<<"$row" \ + && grep -qF 'ACCESSTOKEN=' <<<"$row" \ + && grep -qF 'PGPASSWORD=' <<<"$row" \ + && ! grep -qF "$LEGACY" <<<"$row" && ! grep -qF "$PRIVATE_URL" <<<"$row" \ + && ! grep -qF "$PRIVATE_DSN" <<<"$row" \ + && ! grep -qF "$COMPACT_APIKEY" <<<"$row" \ + && ! grep -qF "$COMPACT_BARE_KEY" <<<"$row" \ + && ! grep -qF "$COMPACT_TOKEN" <<<"$row" \ + && ! grep -qF "$COMPACT_PASSWORD" <<<"$row"; then + ok "shellm-run row masks compact credentials, URL, and DSN --var values" +else + bad "shellm-run row masks compact credentials, URL, and DSN --var values" "redacted fields missing" +fi +if grep -qF -- '--var SECRET_PROBE --var PLAIN=1' <<<"$row" \ + && grep -qF -- "--var CURL_OPTS=$CURL_VALUE" <<<"$row"; then + ok "shellm-run row keeps bare names and CURL_OPTS readable" else - bad "shellm-run row masks credential-looking --var values" "$(printf '%s' "$row" | cut -c1-200)" + bad "shellm-run row keeps bare names and CURL_OPTS readable" "$(printf '%s' "$row" | cut -c1-200)" fi -if grep -qF -- '--var SECRET_PROBE --var PLAIN=1' <<<"$row"; then - ok "shellm-run row keeps the bare name and non-secret vars readable" +if ! grep -qF "$INHERITED_URL" <<<"$row"; then + ok "inherited endpoint is absent from the shellm command record" else - bad "shellm-run row keeps the bare name and non-secret vars readable" "$(printf '%s' "$row" | cut -c1-200)" + bad "inherited endpoint is absent from the shellm command record" fi hits=$(grep -rlF "$LEGACY" "$HEADLONG_HOME" "$WORK/wd" 2>/dev/null | wc -l | tr -d ' ') if [[ "$hits" -eq 0 ]]; then diff --git a/thinkers/_lib/common.sh b/thinkers/_lib/common.sh index a9f0f84..9d80e84 100755 --- a/thinkers/_lib/common.sh +++ b/thinkers/_lib/common.sh @@ -377,6 +377,25 @@ collect_skill_vars() { fi } +# Bare `--var NAME` is resolved from shellm's inherited environment. Export +# skill values in the caller before flag assembly runs in process substitution, +# whose subshell cannot export anything back to its parent. +_export_skill_vars() { + local identity_dir="$1" vname + while IFS= read -r vname; do + [[ -n "$vname" && -n "${!vname:-}" ]] && export "${vname?}" + done < <(collect_skill_vars "$identity_dir") +} + +_export_provider_keys() { + local vname + for vname in ANTHROPIC_API_KEY OPENAI_API_KEY GEMINI_API_KEY OPENROUTER_API_KEY \ + OPENCODE_API_KEY LLM_API_KEY; do + [[ -n "${!vname:-}" ]] && export "${vname?}" + done + return 0 +} + # --------------------------------------------------------------------------- # Path resolution # --------------------------------------------------------------------------- @@ -423,12 +442,13 @@ _build_shellm_flags() { [[ -n "${SHELLM_MODEL:-}" ]] && printf '%s\n' "--var" "SHELLM_MODEL=$SHELLM_MODEL" # The generic openai-compatible provider is env-configured and never # auto-detected, so nested calls need the provider name (routing, not a - # secret) and its key (bare name, like the vendor keys below). + # secret) and key. Endpoint variables are inherited rather than repeated + # as --var: shellm rewrites their values for Docker, and a duplicate extra + # var would overwrite that rewritten value in generated code. [[ -n "${LLM_PROVIDER:-}" ]] && printf '%s\n' "--var" "LLM_PROVIDER=$LLM_PROVIDER" for _ak in ANTHROPIC_API_KEY OPENAI_API_KEY GEMINI_API_KEY OPENROUTER_API_KEY \ OPENCODE_API_KEY LLM_API_KEY; do if [[ -n "${!_ak:-}" ]]; then - export "${_ak?}" printf '%s\n' "--var" "$_ak" fi done @@ -436,8 +456,16 @@ _build_shellm_flags() { # Skill-declared vars while IFS= read -r vname; do [[ -z "$vname" ]] && continue + # shellm owns endpoint alias resolution and Docker rewriting. Emitting + # either alias again as an extra var would overwrite its canonical URL. + case "$vname" in LLM_API_URL|SHELLM_API_URL) continue ;; esac local vval="${!vname:-}" - [[ -n "$vval" ]] && printf '%s\n' "--var" "$vname=$vval" + if [[ -n "$vval" ]]; then + # Skills commonly declare credentials and service endpoints. Keep + # every declared value off argv rather than trying to infer which + # names are sensitive. + printf '%s\n' "--var" "$vname" + fi done < <(collect_skill_vars "$identity_dir") # Standard binaries. Keep this in sync with the tools promised to the diff --git a/thinkers/monolith/step b/thinkers/monolith/step index 524ce43..3233698 100755 --- a/thinkers/monolith/step +++ b/thinkers/monolith/step @@ -247,6 +247,8 @@ goals=$(get_goals "$MEM_DIR") think_prompt=$(load_prompt "$prompt_file" "$IDENTITY_NAME" "$goals") # Build shellm flags (includes traj/mem/chat/etc. in --bin). +_export_provider_keys +_export_skill_vars "$IDENTITY_DIR" shellm_flags=() while IFS= read -r _flag; do [[ -n "$_flag" ]] && shellm_flags+=("$_flag") diff --git a/tools/persona b/tools/persona index d2b5af9..614f406 100755 --- a/tools/persona +++ b/tools/persona @@ -467,7 +467,7 @@ EOF ( set +e; set +o pipefail; _bugreport_text "$iddir" "${big[@]+"${big[@]}"}" ) > "$top/report.txt" 2>&1 || true echo "Scrubbing secrets..." >&2 - _redact_tree "$top" + _redact_tree "$top" || die "bugreport: secret scrubbing failed; no archive was written" tar "${tarq[@]+"${tarq[@]}"}" -czf "$out" -C "$stage" "$(basename "$top")" cat >&2 <`) so two keys can be told apart; passwords and # anything under 20 characters are masked whole. -_redact_tree() { +_redact_tree() ( local root="$1" - local -a pairs=() files=() # pairs: "valuereplacement" - # Literal values: credential-looking names from the env files and the + local -a values=() replacements=() files=() + local tmp="" + trap '[[ -z "$tmp" ]] || rm -f "$tmp"' EXIT + trap 'exit 1' HUP INT TERM + # Literal values: private-looking names from the env files and the # current environment (the env files were loaded into it at startup). local n v e for e in "$SHELLM_HOME/.env" "$APP_DIR/.env"; do @@ -574,13 +577,19 @@ _redact_tree() { _is_secret_name "$n" || continue # shellcheck disable=SC1090 v=$(set -a; . "$e" 2>/dev/null; printf '%s' "${!n}") || continue - [[ "${#v}" -ge 8 ]] && pairs+=("$v $(_redact_hint "$n" "$v")") + if [[ "${#v}" -ge 8 ]]; then + values+=("$v") + replacements+=("$(_redact_hint "$n" "$v")") + fi done < <(sed -n 's/^[[:space:]]*\(export[[:space:]]\{1,\}\)\{0,1\}\([A-Za-z_][A-Za-z0-9_]*\)[[:space:]]*=.*/\2/p' "$e") done while IFS= read -r n; do _is_secret_name "$n" || continue v="${!n:-}" - [[ "${#v}" -ge 8 ]] && pairs+=("$v $(_redact_hint "$n" "$v")") + if [[ "${#v}" -ge 8 ]]; then + values+=("$v") + replacements+=("$(_redact_hint "$n" "$v")") + fi done < <(compgen -e) # Text files only (tar'd dirs can carry binaries in blobs/). while IFS= read -r -d '' f; do files+=("$f"); done \ @@ -588,30 +597,74 @@ _redact_tree() { -o -name '*.txt' -o -name '*.md' -o -name '*.sh' -o -name '*.yaml' -o -name '*.yml' \ -o -name '*.toml' -o -name '*.env' -o -name '*.cfg' -o -name '*.ini' \) -print0 2>/dev/null) [[ "${#files[@]}" -gt 0 ]] || return 0 - # One sed script for everything, ERE (`sed -E` is in both BSD and GNU sed; - # BSD sed has no usable BRE alternation). Write via temp file because - # `sed -i` differs between the two. - local script="" pr - for pr in "${pairs[@]+"${pairs[@]}"}"; do - script+="s/$(_sed_escape "${pr%% *}")/$(_sed_escape "${pr#* }")/g;" - done - # --var NAME=VALUE: KEY/TOKEN names keep a 4+4 hint when long enough, - # other credential names are masked whole. - # (a value starting with "<" is one an earlier pass already replaced) - script+='s/--var ([A-Za-z_]*(KEY|TOKEN)[A-Za-z0-9_]*)=([^ "<\\][^ "\\]{3})[^ "\\]{12,}([^ "\\]{4})/--var \1=/g;' - script+='s/--var ([A-Za-z_]*(KEY|TOKEN|SECRET|PASSW|CREDENTIAL)[A-Za-z0-9_]*)=[^ "<\\][^ "\\]*/--var \1=/g;' - script+='s/sk-([A-Za-z0-9_-]{4})[A-Za-z0-9_-]{12,}([A-Za-z0-9_-]{4})//g;' - script+='s/sk-[A-Za-z0-9_-]{8,}//g;' - local f tmp + # A length-framed stdin stream keeps literals out of argv and the + # filesystem. Python's byte replacement handles embedded newlines and + # regex metacharacters literally; any helper failure aborts the report. + # The --var pass masks through the next --var (or JSON quote/end of line), + # because old unquoted command records cannot distinguish a value with + # spaces from the command words after it. Hiding too much is safer than + # retaining a password suffix. + local f i for f in "${files[@]}"; do - tmp="$f.redact.$$" - if sed -E "$script" "$f" > "$tmp" 2>/dev/null; then + tmp=$(mktemp "$root/.redact-output.XXXXXX") || return 1 + if { + printf '%s\n' "${#values[@]}" + for ((i=0; i<${#values[@]}; i++)); do + LC_ALL=C + printf '%s\n' "${#values[$i]}" + printf '%s\n' "${values[$i]}" + printf '%s\n' "${#replacements[$i]}" + printf '%s\n' "${replacements[$i]}" + done + cat "$f" + } | python3 -c ' +import re, sys + +source = sys.stdin.buffer +def framed(): + size = int(source.readline()) + value = source.read(size) + if source.read(1) != b"\n": + raise ValueError("invalid redaction input") + return value + +pairs = [(framed(), framed()) for _ in range(int(source.readline()))] +data = source.read() +for needle, replacement in pairs: + if needle: + data = data.replace(needle, replacement) + +credential = re.compile(br"(?:SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?|KEY|TOKEN)") +endpoint = re.compile(br"(?:^|_)(?:URL|URI|ENDPOINT|DSN)(?:_|$)") +var = re.compile(br"(--var ([A-Za-z_][A-Za-z0-9_]*)=)(.*?)(?= --var |\"|$)", re.M) +def mask(match): + name = match.group(2).upper() + value = match.group(3) + if value.startswith(b"<") or not (credential.search(name) or endpoint.search(name)): + return match.group(0) + token = value.split(None, 1)[0] + if (b"KEY" in name or b"TOKEN" in name) and not re.search( + br"SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?", name) \ + and not endpoint.search(name) and len(token) >= 20: + replacement = b"" + else: + replacement = b"" + return match.group(1) + replacement + +data = var.sub(mask, data) +data = re.sub(br"sk-([A-Za-z0-9_-]{4})[A-Za-z0-9_-]{12,}([A-Za-z0-9_-]{4})", br"", data) +data = re.sub(br"sk-[A-Za-z0-9_-]{8,}", b"", data) +sys.stdout.buffer.write(data) +' > "$tmp" 2>/dev/null; then mv -f "$tmp" "$f" + tmp="" else rm -f "$tmp" + tmp="" + return 1 fi done -} +) # _redact_hint NAME VALUE — the replacement for VALUE: first/last 4 chars for # keys and tokens of 20+ chars, a plain otherwise. @@ -622,7 +675,10 @@ _upper() { printf '%s' "$1" | tr '[:lower:]' '[:upper:]'; } _redact_hint() { local u v="$2" u=$(_upper "$1") - if [[ "$u" =~ (KEY|TOKEN) && ! "$u" =~ (SECRET|PASSW|CREDENTIAL) && "${#v}" -ge 20 ]]; then + if [[ "$u" =~ (KEY|TOKEN) \ + && ! "$u" =~ (SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?) \ + && ! "$u" =~ (^|_)(URL|URI|ENDPOINT|DSN)(_|$) \ + && "${#v}" -ge 20 ]]; then printf '' "${v:0:4}" "${v: -4}" else printf '' @@ -632,11 +688,8 @@ _redact_hint() { _is_secret_name() { # same notion as shellm's trajectory masking local u u=$(_upper "$1") - [[ "$u" =~ (KEY|TOKEN|SECRET|PASSW|CREDENTIAL) ]] -} - -_sed_escape() { # escape a literal for use in an ERE sed s/// pattern or replacement - printf '%s' "$1" | sed -e 's/[][\/.*^$&+?(){}|\\]/\\&/g' + [[ "$u" =~ (KEY|TOKEN|SECRET|PASSWORD|PASSWD|PASSPHRASE|CREDENTIALS?) \ + || "$u" =~ (^|_)(URL|URI|ENDPOINT|DSN)(_|$) ]] } usage() {