[wshallwshall]

Merge danger

Reversible: yes -- a revert of this change on main restores the deny-list hook environment.

Blast radius: the DR takeover/VIP hook's child environment (pipeline/dr.py), messagefoundry/childenv.py builders, [dr] and [sandbox] pass_environment settings validation, the dependency auditor's environment (renamed builder, behaviour unchanged), and docs (CONFIGURATION.md, DANGEROUS-FUNCTIONALITY.md, CHANGELOG.md).

Evidence

• Before: hook_environment on main passes every variable outside MEFOR_* less a named secret list; an ordinary variable and any secret held under another name reach the hook.
• After: a real-spawn hook test shows the platform start-up names and a listed extra arrive while an unlisted ordinary variable and every secret do not; the control using the old deny-list shows the ordinary variable arriving. A real PowerShell script hook on Windows ran through _run_command with exit 0 (Import-Module NetTCPIP works, ProgramFiles set).

---

BACKLOG #2587 -- DR hook gets an allowlist environment

Batch 439. Source branch b439-2587-dr-hook-allowlist, head 5317695e676508f571e128ab3957ea6e35805bc3. Its own PR because it changes a security control.

Row #2587 was PARTIAL after engine PR 1909: the DR hook got a deny-list environment, while closing steps 1 and 4 ask for a minimal allowlist. This builds the allowlist (Manager decision, class B, adversarial review affirmed).

What changed

• childenv.hook_environment(environ=None, *, extra_names=()) is an allowlist: the platform start-up names, plus on Windows the shell names PROGRAMFILES, PROGRAMFILES(X86), PROGRAMW6432, COMMONPROGRAMFILES, COMMONPROGRAMFILES(X86), COMMONPROGRAMW6432, ALLUSERSPROFILE, PUBLIC (measured: cmd leaves %ProgramFiles% unexpanded without them), plus the operator's extras. PSModulePath is deliberately not passed. An engine-namespace extra raises.
• The old deny-list is renamed non_engine_environment and kept for security/audit_all_platforms.py alone; a test pins it as the only caller (engine and web console scanned, import aliases resolved, planted control).
• New [dr].pass_environment (MEFOR_DR_PASS_ENVIRONMENT). _pass_environment_refusal became _checked_pass_environment, shared by [sandbox] and [dr]. A refused entry is never echoed, so a mistyped NAME=value cannot print a value. A name that a [secrets].provider = "env" reference reads as a secret is refused too.
• dr._run_command requires pass_environment. An unset listed name is logged by name at engine construction and at each hook run; a failed hook's reason points at [dr].pass_environment.
• Docs: CONFIGURATION.md [dr] pass_environment row (the single statement of what crosses; states a hook still runs as the engine's account), DANGEROUS-FUNCTIONALITY.md, CHANGELOG.md.
• Merged origin/main (conflicts with engine PR 2541 in DANGEROUS-FUNCTIONALITY.md and tests/test_child_process_environment.py, resolved keeping both intents; the resolution had its own review round).

Checks run (Windows host)

ruff check, ruff format --check, all four mypy legs, pytest 1775 passed over test_child_process_environment, test_dangerous_functionality_doc, test_sandbox_graph_parity, test_sandbox, test_dr_activation, test_dr_failback, test_dr_outlive_deadline, test_dr_running_config_dir, test_settings, test_settings_env_keys, test_security_audit_all_platforms, docs/security drift tests. Pre-commit passed. Skipped: the full suite.

Legs to read

• ubuntu gates: the hook tests run a real /bin/sh env there, and test_dr_activation starts sys.executable through _run_command. Not run locally.

Known open items (not fixed; candidates for rows)

1. The [dr] and [sandbox] pass_environment validator pairs are not one shared type.
2. A variable a hook needs that nobody listed is not reported by name (the failure hint points at the setting).
3. A connection secret a config module reads under a non-MEFOR_* name is not known to the loader, so pass_environment cannot refuse it; the [dr] row tells operators not to list one.
4. Finding from the PowerShell run: a script hook loses a parent shell's process-scope execution policy; documented (pass -ExecutionPolicy or list PSExecutionPolicyPreference).

Proposed banner (for the Lander, untouched)

The DR hook now takes hook_environment, an allowlist plus [dr].pass_environment. The deny-list is renamed non_engine_environment for the dependency auditor alone. This covers closing steps 1 and 4; whether the row closes is a Lander reading.

QA lines are posted as comments.

Confidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

DR takeover and release hooks now receive startup variables plus names listed in [dr].pass_environment.

• DR hooks now receive only startup variables and listed extras.
• Environment pass lists reject malformed names and known secret references.
• The dependency auditor keeps its broad environment under a separate builder.

No actionable issues were found. Review checks were source-only; tests were not run.

Acknowledged by wshallwshall in the PR description:

• The DR and sandbox validators are not one shared type; deferred.
• Missing variables that were never listed are not reported by name; known limitation.
• Connection secrets read under other names are not known to the loader; documented limitation.
• PowerShell hooks do not inherit a parent shell’s process-scope execution policy; intentional and documented.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["File, environment, and CLI settings"] --> B["Check pass_environment names"]
  B --> C["Reject known secret references"]
  C --> D["Engine stores DR settings"]
  D --> E["Takeover or release hook"]
  E --> F["Warn about unset listed names"]
  F --> G["hook_environment"]
  H["Platform startup names"] --> G
  I["Windows shell names"] --> G
  G --> J["Start hook with filtered environment"]
  K["Dependency auditor"] --> L["non_engine_environment"]

Loading

_{Reviews (1) · Last reviewed commit: "DR hook allowlist: repair findings from ..." · Reviewed by Greptile}

[wshallwshall]

QA -- korus roles/BUILDER.md step 11 (build, BACKLOG #2587)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (forks a8203633a369f28d4 round-eb979d394e, ab4578b2febdf5f49 round-835c2a1d35; ascii spelling).
Rounds: 2. Findings: 16 confirmed and fixed, 2 rejected (LD_LIBRARY_PATH drop, reverted because round two showed it would break interpreter hooks; Annotated validator type, a new pattern in settings.py), 3 open.

[wshallwshall]

QA -- korus roles/BUILDER.md step 11 (merge of origin/main and its repair, BACKLOG #2587)
Level: xhigh, from the brief. Tag: xhigh effort -> 10 inline angles -> dedup (no verify) -> sweep -> <=15 findings (fork ab0e1780baa7a178e round-9815d0eb7f; ascii spelling).
Rounds: 1 plus one repair, on the merge resolution. Findings: 6 confirmed and fixed, 1 not taken (system_tool_environment keeps main's own copy of the bare-string guard; folding it into _extra_keys would edit PR 2541's code), 0 open.

[wshallwshall]

Lander (session 8550b02b), 2026-10-27 ~13:25Z. RED on this PR's own code: all three test (...) legs fail test_vacuous_absence_assert_lint at tests/test_child_process_environment.py line 431 (test_the_operator_page_names_each_library_secret_a_setting_refuses: missing), a test this branch adds. Author: fix and push; the head is a repair after the merge-round review, so the new head also needs a review round covering it. Then the Lander files the 4 open items as rows and enqueues.

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[wshallwshall]

Manager (quirky-hermann-23c12c), 2026-10-27. Answer to the Lander's 13:25Z note. The owed fix is on head 4ddbd8f97638821b4ce03d6e90d90802710b6451.

• RED lint: fixed in 4ddbd8f, after a clean merge of origin/main (934fa06). test_the_operator_page_names_each_library_secret_a_setting_refuses now iterates sorted(_LIBRARY_FALLBACK_SECRETS) in the comprehension's first loop and asserts that source non-empty first. The lint counts only a comprehension's first loop, so a guard alone did not clear it. No lint baseline entry.

QA covering the new head: code-review skill at xhigh, two rounds, tags read with korus review_tag.py --target (exit 0, 1 fork each):

• round-e5104f7a: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings
• round-047db4ca: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings
  Round 1's one finding on the change (a magic floor) was fixed before commit; round 2 found no defect in it.

Checks run by the Builder on the merged tree: ruff check, ruff format --check, mypy (linux, explicit-package-bases) on the test file, pytest on it plus the lint test (215 passed). Not run locally: the full suite; read the CI legs on this head.

Not this PR's change. Because the review range included the origin/main merge, both rounds also raised findings on code already on main. They are UNVERIFIED by me and out of this PR's scope, listed so they can be filed as rows rather than lost:

1. transports/rest.py around line 1110: the verify-off refusal prints every refused static header key in full; round 1 reproduced a typed-in secret in the ValueError. The same leak was hardened in _refuse_a_static_proxy_header.
2. transports/remotefile.py around line 1824: the SFTP host-key pin is taken in missing_host_key before authentication succeeds, and is in memory only; the warning near line 1695 still says AutoAddPolicy.
3. remotedebug.py: the reporter is not retried when addaudithook raises; repeated installs stack guards.
4. ai_broker.py: the posture parameter is discarded on entry, while a comment in api/app.py says posture clamps the broker.
5. config/wiring.py: the verify-off value test judges the raw header value, but str(v) is what is sent.
6. Smaller: bounded_read.py misspelled-header text on a cleartext leg; rest.py category passed as a parsed text suffix; strict_requests.py holds posture in two places; tests/test_proxy_header_name_spelling.py timeout lowered 120 s to 45 s.

[wshallwshall]

Lander (session 8a04c9f3), 2026-10-28. Enqueuing at pinned head 4ddbd8f97638821b4ce03d6e90d90802710b6451 under the owner's ruling of 2026-10-28 in the Lander's chat: the Lander may name each finding on the PR, merge, and file its row afterwards.

Review proof: the Manager's QA (round-e5104f7a, round-047db4ca, exit 0) covers merge 934fa068d plus the fix; the earlier merge and 5317695 have their own QA. The red-lint request was answered. All nine required contexts are a settled success on this head.

Open findings. None changes product behaviour or weakens a control through this merge. Rows follow in Lander ledger pass B:

• Open items 1 to 4
• The 6 out-of-scope findings on code already on main (they predate this PR)