diff --git a/acq.backends/msb.sh b/acq.backends/msb.sh index 18c45ae5..ba1129d0 100644 --- a/acq.backends/msb.sh +++ b/acq.backends/msb.sh @@ -1773,7 +1773,7 @@ EOF # also substitutes the GitHub token on the wire for the API and HTTPS # git-transport hosts bound above, so direct HTTPS clone/push is eligible too. _acq_msb_apply_kit_dir() { - local name="$1" kitdir="$2" + local name="$1" kitdir="$2" mergedn="${3:-}" local spec="${kitdir}/spec.yaml" if [ ! -f "$spec" ]; then echo "acq(msb): kit spec not found: $spec" >&2 @@ -1844,11 +1844,23 @@ EOF # 3) Run commands[]. Reassemble each argv record and exec it as the given uid. # install → run once (idempotent, marker-gated); initFiles/startup → every # apply. msb has no create-time-only hook, so install collapses to a - # marker-gated exec (design §3 lifecycle table). The kit's environment[] - # entries are threaded onto every command as `msb exec -e NAME=value` (msb's - # native per-exec env flag), so the kit's declared guest env is present when - # its lifecycle commands run. - _acq_msb_run_commands "$name" "$spec" + # marker-gated exec (design §3 lifecycle table). + # + # Commands run with the MERGED env of the whole kit set, not just this kit's + # (ADR-0033). Full-set callers pass its array name; the mid-life single-kit + # path (`acq kit apply`) has none, so it recovers the sandbox's persisted + # merged value with THIS kit's entries layered last (last-value-wins). The + # re-layering matters: step 2's append only warns on failure, so the marker + # can be stale — without it the kit's own env would be silently dropped. + local _merged_env=() _merged_envn="$mergedn" + if [ -z "$_merged_envn" ]; then + local _persisted_env=() + _acq_msb_persisted_kit_env_into _persisted_env "$name" + _acq_msb_dedupe_env_into _merged_env \ + ${_persisted_env[@]+"${_persisted_env[@]}"} ${_kit_env[@]+"${_kit_env[@]}"} + _merged_envn=_merged_env + fi + _acq_msb_run_commands "$name" "$spec" "$_merged_envn" } # _acq_msb_reset_kit_env NAME — remove the persisted kit-env marker so a @@ -1972,16 +1984,29 @@ _acq_msb_copy_file_verified() { } # Parse and execute a kit spec's commands[] against sandbox NAME. +# +# Args: NAME SPEC [MERGEDENV_ARRVAR] +# MERGEDENV_ARRVAR names an array of already-merged NAME=value tokens covering the +# FULL effective kit set (_acq_msb_merge_kit_env_into). Omitted, the env falls back +# to this spec's own environment[] — see _acq_msb_apply_kit_dir step 3. _acq_msb_run_commands() { - local name="$1" spec="$2" + local name="$1" spec="$2" mergedn="${3:-}" - # Collect the kit's environment[] entries as `NAME=value` argv-ready tokens, - # threaded onto every command's `msb exec` as `-e NAME=value`. kit_spec_env - # already validates each NAME (^[A-Za-z_][A-Za-z0-9_]*$) and drops unsafe ones, - # so these are safe to pass as exec env. Values are passed as a single argv - # element (never re-split by a shell). + # The guest env threaded onto every command's `msb exec` as `-e NAME=value`. + # Names are already validated by kit_spec_env and re-validated on marker replay; + # each value is one argv element, never re-split by a shell. local _kit_env=() - _acq_msb_collect_kit_env_into _kit_env "$spec" + if [ -n "$mergedn" ]; then + eval "_kit_env=(\${${mergedn}[@]+\"\${${mergedn}[@]}\"})" + else + _acq_msb_collect_kit_env_into _kit_env "$spec" + fi + + # The git guards are owned per kit even though the env is merged — see + # _acq_msb_env_tokens_with_guards_into. + local _own_env=() _own_guard_tok=() + _acq_msb_collect_kit_env_into _own_env "$spec" + _acq_msb_own_guard_tokens_into _own_guard_tok _own_env # Buffer the parsed command stream FIRST, then execute. The exec step calls # `msb exec`, which would consume this loop's stdin if we iterated the heredoc @@ -2009,7 +2034,7 @@ EOF ;; "__END__") reading=0 - _acq_msb_exec_command "$name" "$phase" "$user" "$background" \ + _acq_msb_exec_command "$name" "$phase" "$user" "$background" _own_guard_tok \ ${_kit_env[@]+"${_kit_env[@]}"} -- ${argv[@]+"${argv[@]}"} ;; *) @@ -2056,13 +2081,85 @@ _acq_msb_split_env_argv() { eval "$_argvarr=(\"\$@\")" } -# _acq_msb_exec_flags_into UFLAG_ARRVAR EFLAG_ARRVAR USER KITENV_ARRVAR — build -# the `msb exec` -u/-e flag arrays for one kit command. Maps the sbx uid contract -# (1000/agent) onto our by-name agent user, threads the kit's declared env as -# `-e NAME=value`, and injects git non-interactive guards unless the kit set -# GIT_TERMINAL_PROMPT itself. Arrays passed/returned by name (bash 3.2 compat). +# The three non-interactive git guard vars the adapter injects onto kit lifecycle +# commands (see _acq_msb_env_tokens_with_guards_into). +ACQ_MSB_GIT_GUARD_NAMES="GIT_TERMINAL_PROMPT GIT_ASKPASS SSH_ASKPASS" +# A permanently-empty array used as the default "this kit declared no guard vars" +# argument, so the by-name guard-token parameter is always a real array even when a +# caller omits it (set -u safe). +_acq_msb_no_guards=() + +# _acq_msb_own_guard_tokens_into ARRVAR ENVARR — store, in the array named ARRVAR, +# the guard-named entries of ENVARR (ONE kit spec's own environment[]) as whole +# `NAME=value` tokens. Tokens, not names: see the ownership rule at +# _acq_msb_env_tokens_with_guards_into. +_acq_msb_own_guard_tokens_into() { + local _arrn="$1" _envarrn="$2" _g _ev + eval "$_arrn=()" + eval "set -- \${${_envarrn}[@]+\"\${${_envarrn}[@]}\"}" + for _ev in "$@"; do + for _g in $ACQ_MSB_GIT_GUARD_NAMES; do + case "$_ev" in + "$_g"=*) eval "$_arrn+=(\"\$_ev\")" ;; + esac + done + done +} + +# _acq_msb_env_tokens_with_guards_into ARRVAR ENVARR OWNGUARDARR — store into the +# array named ARRVAR the NAME=value tokens one kit command runs with: the MERGED +# kit env (ENVARR) plus the non-interactive git guards. +# +# GUARD OWNERSHIP (do not widen). A command may only ever receive ITS OWN kit's +# value for a guard name. OWNGUARDARR names THIS kit's own guard tokens. Tracking +# ownership by NAME is not enough: the merge has already collapsed a duplicate +# name to the LAST kit's value, so kit A declaring GIT_TERMINAL_PROMPT=0 and a +# later kit B declaring =1 would leave A's own commands running with B's opt-out, +# and with no ASKPASS guards either. Hence: strip every guard-name token from the +# merged set whoever contributed it, re-add this kit's own, then append acq's +# values unless this kit set GIT_TERMINAL_PROMPT. Closes both collision shapes +# (another kit declaring a guard name this kit does not; another kit declaring the +# same name with a different value) while a kit's own override still stands for +# its own commands. Shared by the exec and staged-script paths, so they cannot +# drift. See ADR-0033. +_acq_msb_env_tokens_with_guards_into() { + local _arrn="$1" _envarrn="$2" _ownarrn="$3" + eval "$_arrn=()" + local _own=() _ev _g _keep + eval "_own=(\${${_ownarrn}[@]+\"\${${_ownarrn}[@]}\"})" + + # 1) The merged env with EVERY guard-name token removed (any kit's value). + eval "set -- \${${_envarrn}[@]+\"\${${_envarrn}[@]}\"}" + for _ev in "$@"; do + _keep=1 + for _g in $ACQ_MSB_GIT_GUARD_NAMES; do + case "$_ev" in "$_g"=*) _keep=0 ;; esac + done + [ "$_keep" -eq 1 ] && eval "$_arrn+=(\"\$_ev\")" + done + + # 2) Re-add only THIS kit's own guard tokens (its intentional override). + for _ev in ${_own[@]+"${_own[@]}"}; do + eval "$_arrn+=(\"\$_ev\")" + done + + # 3) acq's guards, unless this kit set GIT_TERMINAL_PROMPT itself. Appended + # last, so where this kit declared one of the other two the adapter's value + # still wins — the flag order the single-kit path has always emitted. + for _ev in ${_own[@]+"${_own[@]}"}; do + case "$_ev" in GIT_TERMINAL_PROMPT=*) return 0 ;; esac + done + eval "$_arrn+=(\"GIT_TERMINAL_PROMPT=0\" \"GIT_ASKPASS=/bin/false\" \"SSH_ASKPASS=/bin/false\")" +} + +# _acq_msb_exec_flags_into UFLAG_ARRVAR EFLAG_ARRVAR USER KITENV_ARRVAR +# OWNGUARD_ARRVAR — build the `msb exec` -u/-e flag arrays for one kit command. +# Maps the sbx uid contract (1000/agent) onto our by-name agent user and threads +# the env as `-e NAME=value` (merged kit env + git guards, per +# _acq_msb_env_tokens_with_guards_into, which owns the guard rule). OWNGUARD_ARRVAR +# names this kit's own guard tokens. Arrays by name (bash 3.2 compat). _acq_msb_exec_flags_into() { - local _uflag="$1" _eflag="$2" _user="$3" _envarr="$4" + local _uflag="$1" _eflag="$2" _user="$3" _envarr="$4" _ownguards="${5:-_acq_msb_no_guards}" eval "$_uflag=()" eval "$_eflag=()" @@ -2092,29 +2189,18 @@ _acq_msb_exec_flags_into() { ;; esac - # Append the kit's declared guest env as additional `-e NAME=value` flags. + # NON-INTERACTIVE ENFORCEMENT. Kit lifecycle commands run before the agent + # attaches, with no terminal, so they MUST NOT read from a TTY: a kit that + # prompts (e.g. `git clone` of a private repo with no credential -> "Username + # for 'https://github.com':") would BLOCK provision forever — observed, the + # playbook kit hung here. Two defenses: stdin from /dev/null on every kit exec + # (at the call sites), and the GIT_TERMINAL_PROMPT/ASKPASS guards appended below. + local _envtok=() + _acq_msb_env_tokens_with_guards_into _envtok "$_envarr" "$_ownguards" local _ev - eval "set -- \${${_envarr}[@]+\"\${${_envarr}[@]}\"}" - for _ev in "$@"; do + for _ev in ${_envtok[@]+"${_envtok[@]}"}; do eval "$_eflag+=(-e \"\$_ev\")" done - - # NON-INTERACTIVE ENFORCEMENT. Kit lifecycle commands run before the agent - # attaches, with no terminal, so they MUST NOT try to read from a TTY. Docker's - # kit-reference states startup commands are non-interactive; a kit that prompts - # (e.g. `git clone` of a private repo with no credential -> "Username for - # 'https://github.com':") would BLOCK provision forever (observed: the playbook - # kit hung here). Defense-in-depth alongside the kit's own prompt-suppression: - # - stdin from /dev/null for every kit exec, so nothing can read the TTY. - # - GIT_TERMINAL_PROMPT=0 (+ GIT_ASKPASS/SSH_ASKPASS=false) so git fails fast - # instead of prompting. These are injected only if the kit did not already - # set GIT_TERMINAL_PROMPT (kits may override intentionally). - local _kit_env_str - eval "_kit_env_str=\" \${${_envarr}[*]-} \"" - case "$_kit_env_str" in - *" GIT_TERMINAL_PROMPT="*) : ;; - *) eval "$_eflag+=(-e \"GIT_TERMINAL_PROMPT=0\" -e \"GIT_ASKPASS=/bin/false\" -e \"SSH_ASKPASS=/bin/false\")" ;; - esac } # _acq_msb_exec_install NAME USER UFLAG_ARRVAR EFLAG_ARRVAR -- ARGV... — run an @@ -2182,9 +2268,13 @@ _acq_msb_exec_run() { fi } +# Args: NAME PHASE USER BACKGROUND OWNGUARD_ARRVAR [NAME=value ...] -- ARGV... +# OWNGUARD_ARRVAR is the NAME of an array holding this kit's own git-guard tokens +# (see _acq_msb_own_guard_tokens_into); it is passed by name, not by value, because +# a guard VALUE can contain spaces and must never be re-split. _acq_msb_exec_command() { - local name="$1" phase="$2" user="$3" background="$4" - shift 4 + local name="$1" phase="$2" user="$3" background="$4" ownguards="$5" + shift 5 # Split leading NAME=value env tokens (up to the `--` sentinel) from argv. local _kit_env=() _argv=() @@ -2192,9 +2282,8 @@ _acq_msb_exec_command() { [ "${#_argv[@]}" -gt 0 ] || return 0 - # Build the `msb exec` -u/-e flag arrays (uid mapping, kit env, git guards). local uflag=() eflag=() - _acq_msb_exec_flags_into uflag eflag "$user" _kit_env + _acq_msb_exec_flags_into uflag eflag "$user" _kit_env "$ownguards" if [ "$phase" = "install" ]; then _acq_msb_exec_install "$name" "$user" uflag eflag -- "${_argv[@]}" @@ -2247,34 +2336,25 @@ _acq_msb_sq() { printf "'%s'" "${_s//\'/$_q}" } -# _acq_msb_startup_env_prefix_into PREFIX_ARRVAR USER KITENV_ARRVAR — build the -# in-guest `env NAME=value …` prefix tokens for one startup command: HOME for the -# agent user, the kit env vars, and the git non-interactive guards (unless the -# kit set GIT_TERMINAL_PROMPT). Mirrors _acq_msb_exec_flags_into's -e set, but as -# `env` argv rather than `msb exec -e`. Tokens are RAW here (NAME=value); the -# caller single-quote-escapes each before writing it to the script. +# _acq_msb_startup_env_prefix_into PREFIX_ARRVAR USER KITENV_ARRVAR +# OWNGUARD_ARRVAR — build the in-guest `env NAME=value …` prefix tokens for one +# startup command: HOME for the agent user plus the same env set +# _acq_msb_exec_flags_into threads, as `env` argv rather than `msb exec -e`. +# Tokens are RAW here (NAME=value); the caller single-quote-escapes each before +# writing it to the script. _acq_msb_startup_env_prefix_into() { - local _prefixn="$1" _user="$2" _envarrn="$3" + local _prefixn="$1" _user="$2" _envarrn="$3" _ownguards="${4:-_acq_msb_no_guards}" eval "$_prefixn=()" case "$_user" in 1000|agent) eval "$_prefixn+=(\"HOME=/home/agent\")" ;; esac - # Kit-declared env vars (already NAME-validated by kit_spec_env). - local _ev - eval "set -- \${${_envarrn}[@]+\"\${${_envarrn}[@]}\"}" - for _ev in "$@"; do + local _envtok=() _ev + _acq_msb_env_tokens_with_guards_into _envtok "$_envarrn" "$_ownguards" + for _ev in ${_envtok[@]+"${_envtok[@]}"}; do eval "$_prefixn+=(\"\$_ev\")" done - - # Non-interactive git guards, unless the kit set GIT_TERMINAL_PROMPT itself. - local _kit_env_str - eval "_kit_env_str=\" \${${_envarrn}[*]-} \"" - case "$_kit_env_str" in - *" GIT_TERMINAL_PROMPT="*) : ;; - *) eval "$_prefixn+=(\"GIT_TERMINAL_PROMPT=0\" \"GIT_ASKPASS=/bin/false\" \"SSH_ASKPASS=/bin/false\")" ;; - esac } # _acq_msb_startup_emit_command USER BACKGROUND PREFIX_ARRVAR ARGV_ARRVAR — echo @@ -2367,16 +2447,85 @@ $(kit_spec_env "$_spec") EOF } +# _acq_msb_dedupe_env_into ARRVAR TOKEN... — store the NAME=value TOKENs in the +# array named ARRVAR with duplicates collapsed: LAST value for a name wins, FIRST +# appearance fixes the order — the same rule the session replay applies to the +# marker, so a command's env and a session's env agree on which kit's value +# survives. Malformed tokens are dropped; each token stays one array element, so a +# value with spaces or metacharacters is never re-split (SI-10). +_acq_msb_dedupe_env_into() { + local _arrn="$1" + shift + eval "$_arrn=()" + local _line + while IFS= read -r _line; do + [ -n "$_line" ] || continue + eval "$_arrn+=(\"\$_line\")" + done <\n" by name; $'\n' is a literal newline (ANSI-C). @@ -2423,14 +2572,16 @@ EOF [ "$_emitted" -eq 1 ] } -# _acq_msb_generate_startup_script SPEC OUTFILE — write the guest startup script -# for one kit spec's STARTUP-phase commands into OUTFILE. Returns 0 and writes a -# non-empty script iff the kit HAS at least one startup command; returns 1 (and -# writes nothing) when there are none, so the caller registers no empty script. +# _acq_msb_generate_startup_script SPEC OUTFILE [MERGEDENV_ARRVAR] — write the +# guest startup script for one kit spec's STARTUP-phase commands into OUTFILE. +# MERGEDENV_ARRVAR, when given, is the merged whole-kit-set env the body should +# carry (ADR-0033). Returns 0 and writes a non-empty script iff the kit HAS at +# least one startup command; returns 1 (and writes nothing) when there are none, +# so the caller registers no empty script. _acq_msb_generate_startup_script() { - local _spec="$1" _out="$2" _body="" + local _spec="$1" _out="$2" _mergedn="${3:-}" _body="" - _acq_msb_startup_body_into _body "$_spec" || return 1 + _acq_msb_startup_body_into _body "$_spec" "$_mergedn" || return 1 # Emit the file: a self-contained /bin/sh with its own shebang (so --script-path # reads a complete verbatim body; no --shell shebang derivation dependency). @@ -2445,7 +2596,8 @@ _acq_msb_generate_startup_script() { return 0 } -# _acq_msb_stage_startup_script SPEC ARRVAR — generate the startup script for one +# _acq_msb_stage_startup_script SPEC ARRVAR [MERGEDENV_ARRVAR] — generate the +# startup script for one # kit SPEC and, if non-empty, append a `--script-path acq-startup:` # entry into the create-flags array named ARRVAR. The host file is created under # ACQ_MSB_STARTUP_STAGE_DIR (a private 0700 dir under the acq state tree, so kit @@ -2484,7 +2636,7 @@ _acq_msb_generate_startup_script() { # Script at Boot") for the re-verification procedure and remediation. ACQ_MSB_STARTUP_SCRIPT_NAME="acq-startup" _acq_msb_stage_startup_script() { - local _spec="$1" _arrn="$2" + local _spec="$1" _arrn="$2" _mergedn="${3:-}" # One staged script per sandbox provision. If we already staged one, skip # (increment-1 scope; see NOTE above). @@ -2508,7 +2660,7 @@ _acq_msb_stage_startup_script() { } chmod 600 "$_file" 2>/dev/null || true - if _acq_msb_generate_startup_script "$_spec" "$_file"; then + if _acq_msb_generate_startup_script "$_spec" "$_file" "$_mergedn"; then # --script-path names a HOST file native msb reads, so pass the host form # (the shell keeps the POSIX _file for cleanup). See ADR-0029. local _hostfile="$_file" @@ -3010,15 +3162,30 @@ $(kit_spec_published_ports "$spec")" # declare the same path. _volrecs="${_volrecs} $(kit_spec_volumes "$spec")" + done - # Startup-phase commands → a create-time `--script-path acq-startup:` - # (ADR-0017). The script is REGISTERED at create; a bare registration is - # runtime-neutral (staged on the guest PATH, not auto-run at start). Restart - # durability is delivered by the acq `start`/`restart` verb re-running startup - # via the exec heal. install + mid-life apply stay exec-based (see the DESIGN - # NOTE and _acq_msb_apply_kit_dir). Only the first kit with startup commands - # stakes the fixed script name (see _acq_msb_stage_startup_script). - _acq_msb_stage_startup_script "$spec" create_flags + # Merged kit env, computed ONCE now that every kit is fetched and BEFORE any kit + # is applied — see _acq_msb_merge_kit_env_into for why it must be a pre-pass. + # Consumed by the apply loop below and by the staged startup-script body. + local _merged_kit_env=() _pspecs=() _kd + for _kd in ${kitdirs[@]+"${kitdirs[@]}"}; do + _pspecs+=("${_kd}/spec.yaml") + done + _acq_msb_merge_kit_env_into _merged_kit_env ${_pspecs[@]+"${_pspecs[@]}"} + + # Startup-phase commands → a create-time `--script-path acq-startup:` + # (ADR-0017). The script is REGISTERED at create; a bare registration is + # runtime-neutral (staged on the guest PATH, not auto-run at start). Restart + # durability is delivered by the acq `start`/`restart` verb re-running startup + # via the exec heal. install + mid-life apply stay exec-based (see the DESIGN + # NOTE and _acq_msb_apply_kit_dir). Only the first kit with startup commands + # stakes the fixed script name (see _acq_msb_stage_startup_script). + # + # Runs AFTER the fetch loop, not inside it, so the body can carry the merged kit + # env; the same list in the same order keeps the first-kit-stakes rule. + for _kd in ${kitdirs[@]+"${kitdirs[@]}"}; do + [ -f "${_kd}/spec.yaml" ] || continue + _acq_msb_stage_startup_script "${_kd}/spec.yaml" create_flags _merged_kit_env done # CLI host-port overrides (`acq run/create --publish HOST:GUEST`, ADR-0034) are @@ -3614,13 +3781,14 @@ EOF # abort provision under `set -e` and leave an already-created, agent-installed # sandbox half-configured with no diagnostic. Warn and continue — the kits are # individually non-fatal (the playbook kit already self-heals on next start), - # matching acq_backend_ensure_kits_applied's best-effort heal loop. + # matching acq_backend_ensure_kits_applied's best-effort heal loop. Each kit's + # commands get the merged env from the pre-pass above. local kd acq_spin_start "Applying configuration kits" _acq_msb_reset_kit_env "$name" for kd in "${kitdirs[@]}"; do acq_debug "msb provision: applying kit dir $kd ($name)" - if _acq_msb_apply_kit_dir "$name" "$kd"; then + if _acq_msb_apply_kit_dir "$name" "$kd" _merged_kit_env; then acq_debug "msb provision: applied kit dir $kd ($name)" else echo "acq(msb): warning: kit did not fully apply: $kd" >&2 @@ -4271,18 +4439,17 @@ EOF ' /dev/null 2>&1 || true } -# _acq_msb_kit_env_flags_into ARRVAR NAME — build the `-e NAME=value` flag array -# for the kit environment[] entries persisted at /var/lib/acq/kit-env by -# _acq_msb_apply_kit_dir, so every session path (run/attach/shell) sees the env -# the kits declared for agent runtime (see ADR-0011). Empty array when no kit -# declared environment[]. Array passed by name (bash 3.2 compat). +# _acq_msb_persisted_kit_env_into ARRVAR NAME — read the kit environment[] entries +# persisted at /var/lib/acq/kit-env by _acq_msb_apply_kit_dir into the array named +# ARRVAR as NAME=value tokens (see ADR-0011). Empty array when the marker is +# absent or no kit declared environment[]. Array passed by name (bash 3.2 compat). # # The marker is root-owned but its content is kit-derived guest data: re-validate # each NAME (same ^[A-Za-z_][A-Za-z0-9_]*$ charset kit_spec_env enforces) so a # tampered line cannot smuggle an option-shaped or quote-bearing token, and keep # the LAST value for a duplicate name (kits append in application order, so a # later kit overrides an earlier one). -_acq_msb_kit_env_flags_into() { +_acq_msb_persisted_kit_env_into() { local _arrn="$1" _name="$2" eval "$_arrn=()" # Failure-guarded: an absent marker (pre-kit-env sandbox, or no kit declared @@ -4295,7 +4462,7 @@ _acq_msb_kit_env_flags_into() { local _line while IFS= read -r _line; do [ -n "$_line" ] || continue - eval "$_arrn+=(-e \"\$_line\")" + eval "$_arrn+=(\"\$_line\")" done <&2 echo "acq(msb): this apply skips them. Recreate the sandbox (acq rm && acq run) to mount them." >&2 fi + # No merged-env array on purpose: a mid-life apply knows only THIS kit, so + # _acq_msb_apply_kit_dir uses its persisted-marker fallback (see its step 3). _acq_msb_apply_kit_dir "$name" "$kitdir" } +# _acq_msb_heal_kit_set NAME BUILTIN_COUNT KITREF... — the heal's kit loop: fetch +# every ref, merge their environment[] in a pre-pass (see +# _acq_msb_merge_kit_env_into for why fetch and apply must be separate loops), +# reset the marker, then apply each kit with that merged env. Returns 1 if any of +# the first BUILTIN_COUNT refs failed to fetch or apply (the caller's provenance +# verdict), 0 otherwise. _builtin mirrors _dirs so a built-in's apply failure stays +# attributable across the two loops. +_acq_msb_heal_kit_set() { + local name="$1" builtin_count="$2" + shift 2 + local kitref kitdir i=0 ok=1 + local _dirs=() _builtin=() + for kitref in "$@"; do + if kitdir=$(_acq_msb_fetch_kit "$kitref"); then + _dirs+=("$kitdir") + if [ "$i" -lt "$builtin_count" ]; then _builtin+=(1); else _builtin+=(0); fi + else + echo "acq(msb): warning: could not fetch kit for healing: $kitref" >&2 + # A built-in kit that can't even be fetched means we cannot claim the + # bundle is current. Extra-kit fetch failures don't affect the verdict. + [ "$i" -lt "$builtin_count" ] && ok=0 + fi + i=$((i + 1)) + done + local _merged=() _specs=() + for kitdir in ${_dirs[@]+"${_dirs[@]}"}; do + _specs+=("${kitdir}/spec.yaml") + done + _acq_msb_merge_kit_env_into _merged ${_specs[@]+"${_specs[@]}"} + _acq_msb_reset_kit_env "$name" + local _ki + for _ki in ${_dirs[@]+"${!_dirs[@]}"}; do + if ! _acq_msb_apply_kit_dir "$name" "${_dirs[$_ki]}" _merged; then + [ "${_builtin[$_ki]}" -eq 1 ] && ok=0 + fi + done + [ "$ok" -eq 1 ] +} + # --------------------------------------------------------------------------- # acq_backend_ensure_kits_applied — best-effort heal of a sandbox missing kits # --------------------------------------------------------------------------- @@ -5492,23 +5715,9 @@ acq_backend_ensure_kits_applied() { if [ "${#ACQ_CLI_KITS[@]}" -gt 0 ]; then kits+=("${ACQ_CLI_KITS[@]}") fi - local kitref kitdir i=0 ok=1 + local ok=1 acq_spin_start "Refreshing configuration kits" - _acq_msb_reset_kit_env "$name" - for kitref in "${kits[@]}"; do - kitdir=$(_acq_msb_fetch_kit "$kitref") || { - echo "acq(msb): warning: could not fetch kit for healing: $kitref" >&2 - # A built-in kit that can't even be fetched means we cannot claim the - # bundle is current. Extra-kit fetch failures don't affect the verdict. - [ "$i" -lt "$builtin_count" ] && ok=0 - i=$((i + 1)) - continue - } - if ! _acq_msb_apply_kit_dir "$name" "$kitdir"; then - [ "$i" -lt "$builtin_count" ] && ok=0 - fi - i=$((i + 1)) - done + _acq_msb_heal_kit_set "$name" "$builtin_count" "${kits[@]}" || ok=0 acq_spin_stop "Refreshing configuration kits" # Record host-side bundle provenance ONLY when every built-in kit applied. # msb re-applies all built-in kits idempotently, so on full diff --git a/docs/BACKEND_GUIDE.md b/docs/BACKEND_GUIDE.md index 387d7d8f..143fbbb9 100644 --- a/docs/BACKEND_GUIDE.md +++ b/docs/BACKEND_GUIDE.md @@ -1071,7 +1071,13 @@ The neutral vocabulary is: `caps.network.allow`, `files[]`, `commands[]`, `OPENCODE_TUI_CONFIG`, `GITLAB_HOST`). Names must be POSIX identifiers (`^[A-Za-z_][A-Za-z0-9_]*$`; an invalid name is dropped with a warning and reported by `acq kit validate`); values are plain strings. It maps to sbx-v2 -`environment.variables` (synthesized) and to `msb exec -e NAME=value` (per-exec). +`environment.variables` (synthesized) and, on msb, to `msb exec -e NAME=value`. +On both backends these variables are **guest-wide**, not per kit: msb threads the +**merged** set from every applied kit onto every kit's lifecycle commands and +replays the same set on `acq exec`/`acq shell`/attach, so a daemon started by one +kit's `background: true` startup command still sees another kit's config +(last-value-wins for a duplicate name — see +[ADR-0033](adr/0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md)). **Secrets do NOT go here** — use the credential/secret path (`acq secret …`); the kit spec never carries a secret value. diff --git a/docs/adr/0011-msb-backend-and-neutral-kits.md b/docs/adr/0011-msb-backend-and-neutral-kits.md index c7406092..49aef629 100644 --- a/docs/adr/0011-msb-backend-and-neutral-kits.md +++ b/docs/adr/0011-msb-backend-and-neutral-kits.md @@ -412,6 +412,24 @@ strings). The translate layer: > sbx's sandbox-level env semantics. Names are re-validated on replay (tampered > marker defense) and the last value wins for a duplicate name (kits append in > application order, so a later kit overrides an earlier one). +> +> **Update (2026-09-29):** The per-kit scoping of the *command* env above is also +> gone. Widening only the session paths left a gap wherever a kit command +> outlives itself: a `background: true` startup daemon launches agents, and those +> agents inherited only the daemon kit's vars, so another kit's agent +> configuration (including a permission layer) was silently absent. Kit +> `environment[]` is now **guest-wide for kit lifecycle commands too** — every +> `install`/`initFiles`/`startup` command runs with the merged env of the full +> effective kit set, last-value-wins, the same resolution sessions replay. The +> merged set is computed in a **pre-pass over the whole kit set** (so it does not +> depend on kit application order), with the persisted marker as the fallback for +> the mid-life single-kit `acq kit apply` path. The adapter's non-interactive git +> guards stay **owned by the kit whose command is running**: a command receives +> only its own kit's value for a guard name, so another kit can neither disable +> them (by declaring a guard name this kit does not) nor change them (by declaring +> the same name with a different value, which the merge would otherwise resolve in +> the later kit's favor). See +> [ADR-0033](0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md). Deliberately minimal (YAGNI): **static string values only** — no interpolation, no references to `files[]`-staged paths (a kit needing a computed value uses a diff --git a/docs/adr/0017-msb-create-time-startup-script-staging.md b/docs/adr/0017-msb-create-time-startup-script-staging.md index da687532..3b97908b 100644 --- a/docs/adr/0017-msb-create-time-startup-script-staging.md +++ b/docs/adr/0017-msb-create-time-startup-script-staging.md @@ -128,6 +128,27 @@ re-passed on the same invocation stays authoritative. This is consistent with th above: the verb now restores the *full* kit set, not just the built-ins. See `docs/KNOWN_FAILURE_MODES.md` §33. +## Update (2026-09-29): the staged body carries the merged, guest-wide kit env + +Kit `environment[]` became guest-wide for **all** kit lifecycle commands +([ADR-0033](0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md)): each +command runs with the merged env of the full effective kit set rather than only +its own kit's vars. The generated `--script-path` body now emits the same merged +`env NAME=value …` prefix, so the staged path and the exec path cannot drift. + +Two notes specific to this ADR: + +- The staged body is still **inert at boot** (a bare `--script-path` registration + stages the script on the guest PATH; microsandbox does not auto-run it), so this + change fixes no live symptom here. It exists so the re-verification trap above — + a future msb that *does* auto-run a registered script — cannot resurface the + wrong-env bug in a path nobody re-checked. +- The staging call moved **out of** the kit fetch loop in + `acq_backend_provision` into a second loop over the same kit dirs, because the + merged env is only knowable once every kit has been fetched. The + first-kit-with-startup-commands-stakes-the-script-name rule is unchanged + (same list, same order). + --- # ADR-0017: Stage msb kit startup commands as a create-time script for restart durability (original) diff --git a/docs/adr/0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md b/docs/adr/0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md new file mode 100644 index 00000000..0f812d24 --- /dev/null +++ b/docs/adr/0033-msb-kit-env-is-guest-wide-for-lifecycle-commands.md @@ -0,0 +1,117 @@ +--- +title: "msb kit environment[] is guest-wide for kit lifecycle commands" +status: accepted +date: 2026-09-29 +decision_makers: ["Bret Mogilefsky"] +category: architecture +nist_controls: ["AC-3", "AC-6", "CM-2", "CM-6", "CM-7", "SA-8", "SA-15", "SI-10"] +impact_level: low +ato_relevance: no +risk_treatment: mitigate +supersedes: [] +--- + +# ADR-0033: msb kit `environment[]` is guest-wide for kit lifecycle commands + +## Context and Problem Statement + +On msb, a kit's lifecycle commands (`install`, `initFiles`, `startup`) ran with only +**that kit's own** `environment[]`. Sessions were already whole-guest — acq replays +the merged `/var/lib/acq/kit-env` marker on `run`/`attach`/`shell` (ADR-0011's +2026-08-26 update) — but commands were not. + +That split is invisible until a kit's command outlives itself. A daemon kit starts its +supervisor from a `background: true` startup command, so every agent that daemon spawns +inherits only the daemon kit's env: another kit's `OPENCODE_CONFIG` (agent instructions +plus a default-deny permission layer) was silently absent from those agents while the +injected credentials were still present — a quiet loss of a policy control, and +sessions still looked correct, so smoke tests passed. + +**The order-independence trap.** The marker is appended per kit *inside the same loop +that runs that kit's commands*, so "read the merged marker when running commands" is +correct only when the env-declaring kit happens to be applied first, and silently +wrong in the reverse order. Any fix must establish the merged set before the first +kit is applied. + +## Decision Drivers + +- A kit must not silently drop another kit's policy config from the env of agents it + launches, and the result must not depend on kit application order (AC-3, AC-6, CM-6). +- One kit must not weaken another kit's hardening: the non-interactive git guards are + per-kit safety, not shared config (CM-7). +- Kits must not each have to source a marker file; the adapter owns the threading. + +## Considered Options + +**Pre-pass over the full kit set — chosen.** A **per-kit marker read** is +order-dependent by construction (see the trap above), fixing the reported case in one +kit order only. Having each kit source the marker itself is opt-in — a kit that +forgets is silently broken — and pushes an adapter concern into every kit. + +## Decision Outcome + +Kit `environment[]` is **guest-wide configuration**: every kit lifecycle command runs +with the merged env of the full effective kit set, last-value-wins — the same +resolution sessions replay. + +| Path | Source of the merged env | +|------|--------------------------| +| `acq_backend_provision` (create) | pre-pass over every fetched kit spec, before the first apply | +| `acq_backend_ensure_kits_applied` (heal / `start`, `restart`, re-attach) | same pre-pass; refs resolved in one loop, merged, applied in a second | +| `acq_backend_apply_kit` (`acq kit apply`) | no full set exists: the sandbox's persisted marker value, with this kit's own `environment[]` layered last | + +The mid-life path suits an additive add: layering the kit's own entries last means it +still overrides, and a stale or absent marker (the append in the same apply is only +best-effort) can never drop the kit's own env. + +A kit with a `backend_shortcuts.msb` entry is left out of the merge: msb applies it +natively and skips the generic apply, so its `environment[]` never reaches the marker, +and merging it into commands would make them disagree with sessions. + +**Guard ownership is per kit, tracked by value.** acq injects `GIT_TERMINAL_PROMPT=0` + +`GIT_ASKPASS`/`SSH_ASKPASS=/bin/false` onto kit commands unless *that* kit declared +`GIT_TERMINAL_PROMPT` itself; a prompting kit command with no credential blocks +provision forever (observed with the playbook kit). So a command may only ever receive +**its own kit's value** for a guard name — and the ownership record must be that kit's +guard **tokens** (`NAME=value`), not the names it declared, because the merge has +already collapsed a duplicate name to the *last* kit's value: + +> Kit A declares `GIT_TERMINAL_PROMPT=0` and owns a command; a later kit B declares +> `=1`. Under name-only ownership A's own command ran with B's opt-out *and* no +> `GIT_ASKPASS`/`SSH_ASKPASS` at all — worse than before the change, where A correctly +> got its own `0`. + +So: strip every guard-name token from the merged set whoever contributed it, re-add this +kit's own, then append acq's values unless this kit set `GIT_TERMINAL_PROMPT`. Both +collision shapes (name and value) are closed; a kit's own override still stands for its +own commands. The staged `--script-path` body shares the same helper so the paths cannot +drift — **that path is inert today** (ADR-0017), so it fixes no live symptom; it exists +so ADR-0017's re-verification trap cannot resurface this bug unnoticed. + +## Consequences + +- **Broader blast radius per var.** A var one kit declares now reaches every kit's + commands — intended (it matches sbx, where `environment.variables` is sandbox-level), + but a kit author can no longer assume a command's env is theirs alone. +- **Staging-order coupling at create.** `--script-path` staging moved into a second + loop over the same kit dirs, since the merged env is only knowable once every kit is + fetched; same list, same order, so the first-kit-stakes-the-name rule holds. +- **Knowingly left undone: a merged var can name a not-yet-staged path.** Kit *N*'s + commands can now receive e.g. `OPENCODE_CONFIG` pointing at a file kit *N+k* has not + copied in yet, turning "no team config" into "possibly broken team config" until the + declaring kit's files land. That is the startup-races-its-own-prerequisites class, not + an env-scoping one: fixing it needs an ordering/barrier decision (stage all `files[]` + before any commands, or a readiness barrier before background startup). + +## Links + +- ADR-0011 (the `environment` vocabulary and session-replay marker this widens), + ADR-0014 (`background: true`, whose daemons made the gap visible), ADR-0017 (the + staged-script path and its inertness caveat). +- `acq.backends/msb.sh`: `_acq_msb_merge_kit_env_into` (pre-pass), + `_acq_msb_own_guard_tokens_into` + `_acq_msb_env_tokens_with_guards_into` (guard + ownership), `_acq_msb_persisted_kit_env_into` (mid-life fallback, session replay). +- Tests: `test/bats/79-msb-merged-kit-env.bats` (lifecycle commands), + `78-msb-kit-env.bats` (marker/session replay), `73-msb-kits-startup.bats` (staged body). +- Reported as GSA-TTS/agentic-coding-quickstart#515; the deferred + startup-races-its-prerequisites class is GSA-TTS/agentic-coding-quickstart#506. diff --git a/test/bats/73-msb-kits-startup.bats b/test/bats/73-msb-kits-startup.bats index 6cc39853..fce33281 100644 --- a/test/bats/73-msb-kits-startup.bats +++ b/test/bats/73-msb-kits-startup.bats @@ -475,3 +475,130 @@ SPEC assert_regex "$body" 'echo NAMED_USER_MARKER' refute_regex "$body" 'runuser -u agent' } + +@test "0017 + ADR-0033: the staged startup body carries the MERGED kit env, guards per kit" { + # The staged --script-path body must scope env the same way the exec path does: + # merged across the whole kit set, but the git-guard decision kept per kit. This + # path is INERT today (a bare --script-path registration is staged on the guest + # PATH, not auto-run at boot — see ADR-0017), so fixing it does not fix the + # reported daemon symptom; it keeps the two paths from drifting, so the ADR-0017 + # re-verification trap (a future msb that DOES auto-run the script) cannot turn + # into a live instance of the same bug. + : > "$CALLS" + run bash -c ' + export ACQ_SECRET_STORE_DIR="'"$STUBDIR"'/mergestage-secrets" + export ACQ_MSB_KEEP_STARTUP_STAGE=1 ACQ_MSB_STARTUP_STAGE_DIR="'"$STUBDIR"'/mergestage-stage" + . "'"$REPO_ROOT"'/acq.backends/secret-store.sh" + . "'"$REPO_ROOT"'/acq.backends/kit-translate.sh" + . "'"$REPO_ROOT"'/acq.backends/msb.sh" + # The kit with the startup command is fetched FIRST (it stakes the single + # script name), and the kit declaring the env comes LATER — the order that + # fails without a pre-pass over the full kit set. + sk="'"$STUBDIR"'/stagestartupkit"; mkdir -p "$sk" + cat >"$sk/spec.yaml" <<'"'"'SPEC'"'"' +schemaVersion: "hybrid/v1" +kind: mixin +name: stage-startup-kit +displayName: Stage Startup Kit +description: owns the staged startup command and opts out of the git guards +environment: + GIT_TERMINAL_PROMPT: "1" +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo STAGED_MERGED_MARKER +SPEC + ck="'"$STUBDIR"'/stageconfigkit"; mkdir -p "$ck" + cat >"$ck/spec.yaml" <<'"'"'SPEC'"'"' +schemaVersion: "hybrid/v1" +kind: mixin +name: stage-config-kit +displayName: Stage Config Kit +description: declares agent-runtime config a later-merged body must carry +environment: + OPENCODE_CONFIG: /home/agent/.config/opencode/team.jsonc +SPEC + nk="'"$STUBDIR"'/stagenokit"; mkdir -p "$nk" + printf "schemaVersion: \"hybrid/v1\"\nkind: mixin\nname: x\ndisplayName: X\ndescription: x\n" > "$nk/spec.yaml" + ZSCALER_KIT=k1; USAI_KIT=k2; PLAYBOOK_KIT=k3; GITSSHSIGN_KIT=k4 + _acq_msb_fetch_kit() { + case "$1" in k1) printf "%s\n" "$sk" ;; k2) printf "%s\n" "$ck" ;; *) printf "%s\n" "$nk" ;; esac + } + acq_backend_provision stagemergebox shell /tmp >/dev/null 2>&1 + ' + local body; body=$(_staged_body mergestage-stage) + assert_regex "$body" 'echo STAGED_MERGED_MARKER' + # The other kit's var reaches this kit's staged command. + assert_regex "$body" "'OPENCODE_CONFIG=/home/agent/.config/opencode/team.jsonc'" + # This kit opted out of the guards, so its own value stands and acq's does not + # appear alongside it. + assert_regex "$body" "'GIT_TERMINAL_PROMPT=1'" + refute_regex "$body" 'GIT_TERMINAL_PROMPT=0' + # Still a valid, busybox-safe /bin/sh body. + refute_regex "$body" 'env --' + local f; f=$(find "$STUBDIR/mergestage-stage" -type f 2>/dev/null | head -n1) + run sh -n "$f" + assert_success +} + +@test "0017 + ADR-0033: a staged body keeps its OWN kit's guard value, not a later kit's" { + # Value-collision on the staged path: the kit that stakes the script declares + # GIT_TERMINAL_PROMPT=0 (the safe value); a later-merged kit declares =1. The + # body must carry the staking kit's own value. Same defect class as the exec + # path — the merge collapses a duplicate name to the LAST kit's value, so guard + # ownership has to be tracked by value, not by name. + run bash -c ' + export ACQ_SECRET_STORE_DIR="'"$STUBDIR"'/gvstage-secrets" + export ACQ_MSB_KEEP_STARTUP_STAGE=1 ACQ_MSB_STARTUP_STAGE_DIR="'"$STUBDIR"'/gvstage-stage" + . "'"$REPO_ROOT"'/acq.backends/secret-store.sh" + . "'"$REPO_ROOT"'/acq.backends/kit-translate.sh" + . "'"$REPO_ROOT"'/acq.backends/msb.sh" + sk="'"$STUBDIR"'/gvstagekit"; mkdir -p "$sk" + cat >"$sk/spec.yaml" <<'"'"'SPEC'"'"' +schemaVersion: "hybrid/v1" +kind: mixin +name: gv-stage-kit +displayName: GV Stage Kit +description: stakes the staged script and declares the safe guard value +environment: + GIT_TERMINAL_PROMPT: "0" +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo GV_STAGED_MARKER +SPEC + bk="'"$STUBDIR"'/gvoptoutkit"; mkdir -p "$bk" + cat >"$bk/spec.yaml" <<'"'"'SPEC'"'"' +schemaVersion: "hybrid/v1" +kind: mixin +name: gv-optout-kit +displayName: GV OptOut Kit +description: a later kit opting ITSELF out, plus a shared non-guard var +environment: + GIT_TERMINAL_PROMPT: "1" + TEAM_VAR: shared +SPEC + nk="'"$STUBDIR"'/gvnokit"; mkdir -p "$nk" + printf "schemaVersion: \"hybrid/v1\"\nkind: mixin\nname: x\ndisplayName: X\ndescription: x\n" > "$nk/spec.yaml" + ZSCALER_KIT=k1; USAI_KIT=k2; PLAYBOOK_KIT=k3; GITSSHSIGN_KIT=k4 + _acq_msb_fetch_kit() { + case "$1" in k1) printf "%s\n" "$sk" ;; k2) printf "%s\n" "$bk" ;; *) printf "%s\n" "$nk" ;; esac + } + acq_backend_provision gvstagebox shell /tmp >/dev/null 2>&1 + ' + local body; body=$(_staged_body gvstage-stage) + assert_regex "$body" 'echo GV_STAGED_MARKER' + assert_regex "$body" "'GIT_TERMINAL_PROMPT=0'" + refute_regex "$body" 'GIT_TERMINAL_PROMPT=1' + # The other kit's NON-guard var still reaches the body (merge still applies). + assert_regex "$body" "'TEAM_VAR=shared'" + local f; f=$(find "$STUBDIR/gvstage-stage" -type f 2>/dev/null | head -n1) + run sh -n "$f" + assert_success +} diff --git a/test/bats/78-msb-kit-env.bats b/test/bats/78-msb-kit-env.bats index d02282da..dd48882b 100644 --- a/test/bats/78-msb-kit-env.bats +++ b/test/bats/78-msb-kit-env.bats @@ -70,7 +70,11 @@ SPEC _acq_msb_apply_kit_dir envbox "$nk" ' assert_success - refute_regex "$(cat "$CALLS")" '/var/lib/acq/kit-env' + # Asserts the WRITE specifically (`>> /var/lib/acq/kit-env`), not any mention of + # the path: a kit with no environment[] of its own still READS the merged marker + # so its commands inherit the other kits' env (see ADR-0033) — that read is + # expected here, an append is not. + refute_regex "$(cat "$CALLS")" '>> /var/lib/acq/kit-env' } @test "msb kit env: acq exec replays persisted entries as -e flags; none when marker empty" { diff --git a/test/bats/79-msb-merged-kit-env.bats b/test/bats/79-msb-merged-kit-env.bats new file mode 100644 index 00000000..fa404c4d --- /dev/null +++ b/test/bats/79-msb-merged-kit-env.bats @@ -0,0 +1,431 @@ +#!/usr/bin/env bats +# +# 79-msb-merged-kit-env.bats — msb kit environment[] is guest-wide for kit +# lifecycle commands (ADR-0033). +# +# Companion to 78-msb-kit-env.bats, which covers the environment[] marker and its +# SESSION replay. This file covers the LIFECYCLE-COMMAND side: the merged +# whole-kit-set env threaded onto install/initFiles/startup, its order +# independence, duplicate resolution, the mid-life and heal paths, and the +# per-kit ownership of the non-interactive git guards. +# +# shellcheck shell=bats + +setup() { acq_setup_stubs; load_acq; } +teardown() { acq_teardown_stubs; } + +load 'helper' + +# Kit env is guest-wide guest configuration, but on msb a kit's lifecycle +# commands used to run with only THAT kit's environment[]. The reported symptom: +# a `background: true` startup daemon (the Paseo kit's `paseo daemon`) launches +# agents, and those agents inherited only the daemon kit's vars — a team kit's +# OPENCODE_CONFIG (team instructions + a default-deny permission layer) was +# silently absent while the secrets were still present. The fix threads the +# MERGED env of the full effective kit set onto every lifecycle command. + +# _env_merge_provision KITDIRS... — provision `mergebox` with the given local kit +# dirs as the full built-in kit set, in the order given (so the same two kits can +# be applied in either order). Echoes nothing; assertions read $CALLS. +_env_merge_provision() { + : > "$CALLS" + local a="$1" b="$2" + run bash -c ' + a="$1"; b="$2" + export ACQ_SECRET_STORE_DIR="'"$STUBDIR"'/merge-secrets" + export ACQ_MSB_STARTUP_STAGE_DIR="'"$STUBDIR"'/merge-stage" + . "'"$REPO_ROOT"'/acq.backends/secret-store.sh" + . "'"$REPO_ROOT"'/acq.backends/kit-translate.sh" + . "'"$REPO_ROOT"'/acq.backends/msb.sh" + ZSCALER_KIT=k1; USAI_KIT=k2; PLAYBOOK_KIT=k3; GITSSHSIGN_KIT=k4 + nk="'"$STUBDIR"'/mergenokit"; mkdir -p "$nk" + printf "schemaVersion: \"hybrid/v1\"\nkind: mixin\nname: x\ndisplayName: X\ndescription: x\n" > "$nk/spec.yaml" + _acq_msb_fetch_kit() { + case "$1" in k1) printf "%s\n" "$a" ;; k2) printf "%s\n" "$b" ;; *) printf "%s\n" "$nk" ;; esac + } + acq_backend_provision mergebox shell /tmp >/dev/null 2>&1 + ' _ "$a" "$b" +} + +# Write a kit dir that declares OPENCODE_CONFIG and has no commands. +_env_kit_config() { + local d="$1"; mkdir -p "$d" + cat > "$d/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: team-kit +displayName: Team Kit +description: declares agent-runtime config other kits' commands must see +environment: + OPENCODE_CONFIG: /home/agent/.config/opencode/team.jsonc +SPEC +} + +# Write a kit dir whose commands span all three lifecycle phases, the startup one +# detached (the reported daemon shape). +_env_kit_daemon() { + local d="$1"; mkdir -p "$d" + cat > "$d/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: daemon-kit +displayName: Daemon Kit +description: install + initFiles + a background startup daemon +environment: + PASEO_OWN_VAR: own +commands: + - phase: install + user: "0" + command: + - sh + - -c + - echo INSTALL_CMD + - phase: initFiles + user: "0" + command: + - sh + - -c + - echo INITFILES_CMD + - phase: startup + user: "0" + background: true + command: + - paseo-daemon +SPEC +} + +@test "ADR-0033: merged env reaches a background startup daemon and every phase" { + _env_kit_config "$STUBDIR/cfgkit" + _env_kit_daemon "$STUBDIR/daemonkit" + _env_merge_provision "$STUBDIR/cfgkit" "$STUBDIR/daemonkit" + local log; log=$(cat "$CALLS") + # The detached daemon exec carries the OTHER kit's var, not just its own. + local bg; bg=$(printf '%s\n' "$log" | grep 'nohup' | head -n1) + assert_regex "$bg" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' + assert_regex "$bg" '\-e PASEO_OWN_VAR=own' + # install and initFiles too — the same guest-wide env contract. + local inst; inst=$(printf '%s\n' "$log" | grep 'echo INSTALL_CMD' | head -n1) + assert_regex "$inst" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' + local init; init=$(printf '%s\n' "$log" | grep 'echo INITFILES_CMD' | head -n1) + assert_regex "$init" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' +} + +@test "ADR-0033: the merged env is the same in BOTH kit application orders" { + _env_kit_config "$STUBDIR/cfgkit" + _env_kit_daemon "$STUBDIR/daemonkit" + # config kit applied FIRST (the order a per-kit marker read would also pass). + _env_merge_provision "$STUBDIR/cfgkit" "$STUBDIR/daemonkit" + local bg1; bg1=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg1" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' + # daemon kit applied FIRST — the order that silently fails without a pre-pass: + # the marker does not yet hold the config kit's entry when the daemon launches. + _env_merge_provision "$STUBDIR/daemonkit" "$STUBDIR/cfgkit" + local bg2; bg2=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg2" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' +} + +@test "ADR-0033: a duplicate name resolves last-kit-wins on commands" { + local k1="$STUBDIR/dupkit1" k2="$STUBDIR/dupkit2" + mkdir -p "$k1" "$k2" + cat > "$k1/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: dup-one +displayName: Dup One +description: sets GITLAB_HOST first and owns the command +environment: + GITLAB_HOST: gitlab.first.gov +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo DUP_CMD +SPEC + cat > "$k2/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: dup-two +displayName: Dup Two +description: overrides GITLAB_HOST from a later kit, and carries an awkward value +environment: + GITLAB_HOST: gitlab.second.gov + OPENCODE_CONFIG: /home/agent/a b.jsonc; touch /tmp/MERGE_PWNED +SPEC + _env_merge_provision "$k1" "$k2" + local cmd; cmd=$(printf '%s\n' "$(cat "$CALLS")" | grep 'echo DUP_CMD' | head -n1) + # The command belongs to the EARLIER kit, yet must see the LATER kit's value — + # the same resolution the session replay applies, and exactly once (never both). + assert_regex "$cmd" '\-e GITLAB_HOST=gitlab\.second\.gov' + refute_regex "$cmd" 'gitlab\.first\.gov' + # A value with spaces and metacharacters crosses the merge as ONE token: it is + # threaded as a single `-e` argv element, never re-split into a second variable + # and never interpreted as shell syntax (SI-10). + assert_regex "$cmd" '\-e OPENCODE_CONFIG=/home/agent/a b\.jsonc; touch /tmp/MERGE_PWNED' + refute_regex "$cmd" '\-e touch' +} + +# --- the git guards belong to the kit whose command is running ------------ +# +# Widening the threaded env makes a whole failure class reachable: if the MERGED +# set were also used to decide the guards, one kit could disable +# GIT_TERMINAL_PROMPT=0/GIT_ASKPASS/SSH_ASKPASS for every other kit's commands, +# and a prompting kit command blocks provision forever (the adapter's own comment +# records the playbook kit hanging there). The next three tests pin the two ways +# a kit could reach another kit's guards — NAME collision and VALUE collision — +# plus the case that must keep working: a kit's own intentional override. + +# NAME collision. Passes at base too (each kit then saw only its own env, so the +# question could not arise); it guards the widening rather than reproducing a bug. +@test "ADR-0033: one kit's GIT_TERMINAL_PROMPT does not strip another kit's guards" { + local gk="$STUBDIR/gitpromptkit" pk="$STUBDIR/plainkit" + mkdir -p "$gk" "$pk" + # A kit that deliberately opts out of the non-interactive git guards. + cat > "$gk/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: prompt-kit +displayName: Prompt Kit +description: opts itself out of the git guards +environment: + GIT_TERMINAL_PROMPT: "1" +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo PROMPT_KIT_CMD +SPEC + # A different kit that did NOT opt out: its commands must keep the guards, or a + # prompting command would block provision forever. + cat > "$pk/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: plain-kit +displayName: Plain Kit +description: relies on the adapter's git guards +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo PLAIN_KIT_CMD +SPEC + _env_merge_provision "$gk" "$pk" + local log; log=$(cat "$CALLS") + local plain; plain=$(printf '%s\n' "$log" | grep 'echo PLAIN_KIT_CMD' | head -n1) + assert_regex "$plain" '\-e GIT_TERMINAL_PROMPT=0' + assert_regex "$plain" '\-e GIT_ASKPASS=/bin/false' + assert_regex "$plain" '\-e SSH_ASKPASS=/bin/false' + refute_regex "$plain" 'GIT_TERMINAL_PROMPT=1' + # The opting-out kit still gets its own value, and only that value. + local prompt; prompt=$(printf '%s\n' "$log" | grep 'echo PROMPT_KIT_CMD' | head -n1) + assert_regex "$prompt" '\-e GIT_TERMINAL_PROMPT=1' + refute_regex "$prompt" 'GIT_TERMINAL_PROMPT=0' +} + +@test "ADR-0033: mid-life 'kit apply' recovers the merged env from the marker" { + # A mid-life single-kit apply has NO full kit set to pre-merge, so it must fall + # back to the env the sandbox already persisted — otherwise adding a daemon kit + # to a live sandbox reintroduces exactly the reported bug for that kit. + printf 'OPENCODE_CONFIG=/home/agent/.config/opencode/team.jsonc\n' > "$STUBDIR/.kit_env" + local mk="$STUBDIR/midlifekit"; mkdir -p "$mk" + cat > "$mk/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: midlife-kit +displayName: Midlife Kit +description: added to a live sandbox; its own env plus what is already persisted +environment: + MIDLIFE_OWN: yes +commands: + - phase: startup + user: "0" + background: true + command: + - midlife-daemon +SPEC + : > "$CALLS" + ( export ACQ_SECRET_STORE_DIR="$STUBDIR/midlife-secrets" + . "${REPO_ROOT}/acq.backends/secret-store.sh" + . "${REPO_ROOT}/acq.backends/kit-translate.sh" + . "${REPO_ROOT}/acq.backends/msb.sh" + acq_backend_apply_kit midlifebox "$mk" >/dev/null 2>&1 ) + local bg; bg=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' + assert_regex "$bg" '\-e MIDLIFE_OWN=yes' +} + +@test "ADR-0033: the heal path also merges, daemon kit applied first" { + # The heal (acq start/restart, re-attach) is the OTHER full-set path, and it is + # the one that restarts a kit's daemon on resume. Same pre-pass requirement: the + # daemon kit is healed FIRST here, so a per-kit marker read would give it an env + # missing the config kit's var. + printf 'healmergebox\n' > "$STUBDIR/.msb_sandbox_list" + printf 'healmergebox\n' > "$STUBDIR/.msb_running_list" + _env_kit_config "$STUBDIR/cfgkit" + _env_kit_daemon "$STUBDIR/daemonkit" + : > "$CALLS" + ( export ACQ_SECRET_STORE_DIR="$STUBDIR/healmerge-secrets" + export ACQ_MSB_STARTUP_STAGE_DIR="$STUBDIR/healmerge-stage" + . "${REPO_ROOT}/acq.backends/secret-store.sh" + . "${REPO_ROOT}/acq.backends/kit-translate.sh" + . "${REPO_ROOT}/acq.backends/msb.sh" + # shellcheck disable=SC2034 # read by the sourced acq_backend_ensure_kits_applied + ACQ_CLI_KITS=() + nk="$STUBDIR/healmergenokit"; mkdir -p "$nk" + printf 'schemaVersion: "hybrid/v1"\nkind: mixin\nname: x\ndisplayName: X\ndescription: x\n' > "$nk/spec.yaml" + # Resolve by CALL ORDER rather than by kit ref, so the built-in kit refs stay + # untouched: the heal fetches them in list order, so call 1 is the daemon kit + # and call 2 the config kit — the daemon applied FIRST. + printf 0 > "$STUBDIR/.healmerge_n" + _acq_msb_fetch_kit() { + local n; n=$(cat "$STUBDIR/.healmerge_n") + printf '%s' "$((n + 1))" > "$STUBDIR/.healmerge_n" + case "$n" in + 0) printf '%s\n' "$STUBDIR/daemonkit" ;; + 1) printf '%s\n' "$STUBDIR/cfgkit" ;; + *) printf '%s\n' "$nk" ;; + esac + } + acq_backend_ensure_kits_applied healmergebox >/dev/null 2>&1 ) + local bg; bg=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg" '\-e OPENCODE_CONFIG=/home/agent/\.config/opencode/team\.jsonc' + assert_regex "$bg" '\-e PASEO_OWN_VAR=own' +} + + +# VALUE collision — the case a name-only ownership record cannot catch. The merge +# collapses a duplicate name to the LAST kit's value, so an earlier kit that +# declared GIT_TERMINAL_PROMPT=0 saw kit B's =1 on its own commands, with no +# GIT_ASKPASS/SSH_ASKPASS added either (the "owner declared it, leave it alone" +# branch). Reproduces a real regression: at base that command got its own 0. +# +# The two kits also cover the mirror requirement in one pass: kit B is a kit that +# deliberately opts ITSELF out, so its own override must survive intact (its value, +# exactly once, no acq-injected 0 alongside it) and both kits must still receive +# the other's NON-guard merged vars — proving the hole was closed by scoping the +# guards, not by disabling the merge. +@test "ADR-0033: two kits declaring the SAME guard name with different values" { + local ka="$STUBDIR/guardvalkitA" kb="$STUBDIR/guardvalkitB" + mkdir -p "$ka" "$kb" + cat > "$ka/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: guardval-a +displayName: GuardVal A +description: owns a command and declares the safe guard value +environment: + GIT_TERMINAL_PROMPT: "0" + A_VAR: from-a +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo GUARDVAL_A_CMD +SPEC + cat > "$kb/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: guardval-b +displayName: GuardVal B +description: a later kit opting ITSELF out of the guards +environment: + GIT_TERMINAL_PROMPT: "1" + B_VAR: from-b +commands: + - phase: startup + user: "0" + command: + - sh + - -c + - echo GUARDVAL_B_CMD +SPEC + _env_merge_provision "$ka" "$kb" + local log a b na nb + log=$(cat "$CALLS") + # Kit A's command: its own value, never kit B's, and the ASKPASS guards absent + # because A declared GIT_TERMINAL_PROMPT itself. + a=$(printf '%s\n' "$log" | grep 'echo GUARDVAL_A_CMD' | head -n1) + assert_regex "$a" '\-e GIT_TERMINAL_PROMPT=0' + refute_regex "$a" 'GIT_TERMINAL_PROMPT=1' + na=$(printf '%s\n' "$a" | tr ' ' '\n' | grep -c '^GIT_TERMINAL_PROMPT=') + assert_equal "$na" "1" + # Kit B's command: its own opt-out value stands, exactly once, with no + # acq-injected 0 — a kit's intentional override is not clobbered. + b=$(printf '%s\n' "$log" | grep 'echo GUARDVAL_B_CMD' | head -n1) + assert_regex "$b" '\-e GIT_TERMINAL_PROMPT=1' + refute_regex "$b" 'GIT_TERMINAL_PROMPT=0' + nb=$(printf '%s\n' "$b" | tr ' ' '\n' | grep -c '^GIT_TERMINAL_PROMPT=') + assert_equal "$nb" "1" + # Both still receive the OTHER kit's non-guard merged vars: the guards were + # scoped, the merge was not disabled. + assert_regex "$a" '\-e B_VAR=from-b' + assert_regex "$b" '\-e A_VAR=from-a' +} + +@test "ADR-0033: mid-life 'kit apply' keeps its own env when the marker append is lost" { + # The persisted read can return a STALE non-empty marker that lacks this kit's + # entries (step 2's append failed: it only warns). The kit's commands must still + # carry its own environment[], winning over a stale value for the same name, + # while keeping the other persisted entries. + export STUB_RECORDED_KIT_ENV='OPENCODE_CONFIG=/stale.jsonc +OTHER_KIT_VAR=kept' + local mk="$STUBDIR/lostappendkit"; mkdir -p "$mk" + cat > "$mk/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: lostappend-kit +displayName: Lost Append Kit +description: added to a live sandbox whose marker append does not land +environment: + OPENCODE_CONFIG: /fresh.jsonc + LOST_OWN: yes +commands: + - phase: startup + user: "0" + background: true + command: + - lostappend-daemon +SPEC + : > "$CALLS" + ( export ACQ_SECRET_STORE_DIR="$STUBDIR/lostappend-secrets" + . "${REPO_ROOT}/acq.backends/secret-store.sh" + . "${REPO_ROOT}/acq.backends/kit-translate.sh" + . "${REPO_ROOT}/acq.backends/msb.sh" + acq_backend_apply_kit lostappendbox "$mk" >/dev/null 2>&1 ) + local bg; bg=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg" '\-e LOST_OWN=yes' + assert_regex "$bg" '\-e OPENCODE_CONFIG=/fresh\.jsonc' + refute_regex "$bg" 'stale\.jsonc' + assert_regex "$bg" '\-e OTHER_KIT_VAR=kept' +} + +@test "ADR-0033: a backend-shortcut kit's environment[] is not merged into other kits" { + # A kit with a backend_shortcuts.msb entry skips the generic apply path, so its + # environment[] is never persisted for sessions. Merging it into other kits' + # commands would make commands and sessions disagree on the guest env. + local sk="$STUBDIR/shortcutkit"; mkdir -p "$sk" + cat > "$sk/spec.yaml" <<'SPEC' +schemaVersion: "hybrid/v1" +kind: mixin +name: shortcut-kit +displayName: Shortcut Kit +description: handled natively by msb; declares env that must stay unmerged +environment: + SHORTCUT_VAR: leaked +backend_shortcuts: + msb: + trust_host_cas: true +SPEC + _env_kit_daemon "$STUBDIR/daemonkit" + _env_merge_provision "$sk" "$STUBDIR/daemonkit" + local bg; bg=$(printf '%s\n' "$(cat "$CALLS")" | grep 'nohup' | head -n1) + assert_regex "$bg" '\-e PASEO_OWN_VAR=own' + refute_regex "$bg" 'SHORTCUT_VAR' +}