Commit graph

7 commits

Author SHA1 Message Date
Parideboy
c3c921f2f7
test(install/windows): verify the PATH guard against the real HKCU registry (#3068)
## Description

Follow-up requested in review of #2972, on top of the merged fix for
#2970 (#2985). Test-only; no
production code is touched and the `HEADROOM_INSTALL_PATH_SCOPE`
mechanism is unchanged.

`test_powershell_installer_does_not_leak_into_user_path` currently
guards the fix by comparing the
entry count of `[Environment]::GetEnvironmentVariable('Path','User')`
across an installer run. That
infers success from the environment variable rather than verifying it,
and it leaves three gaps:

- The .NET getter expands `%USERPROFILE%`-style references, so it cannot
observe a change of the
  registry value kind (`REG_EXPAND_SZ` vs `REG_SZ`) at all.
- A count comparison passes when an entry is replaced or reordered
rather than appended.
- There is no restore path. If the guard regresses, the test reports the
leak and then leaves the
polluted value behind in the contributor's registry, which is precisely
the damage #2970
  described: the test that detects the pollution also causes it.

This PR reads `HKCU\Environment` directly instead, so the assertion
verifies the guard rather than
assuming it.

## Type of Change

- [ ] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [x] Code refactoring (no functional changes)

## Changes Made

- `tests/test_install/test_native_installers.py`: new
`_read_user_path_entry` helper returning the
raw `HKCU\Environment` `Path` value together with its registry kind (or
`None` when the value is
absent), and `_restore_user_path_entry` writing that exact value and
kind back. Both import
`winreg` inside the function body, so the module still imports on
non-Windows hosts.
- `tests/test_install/test_native_installers.py`:
`test_powershell_installer_does_not_leak_into_user_path`
now records the raw value before the run and asserts both that the
throwaway install dir is absent
from the value afterwards (naming the #2970 symptom in the failure
message) and that value and
kind are byte-identical. The PowerShell subprocess that counted PATH
entries is gone, so the test
  also spawns one process fewer.
- `tests/test_install/test_native_installers.py`: the test now runs
under `try/finally`. The
`finally` cleans up the fake docker state, which this test was missing
relative to its sibling

`test_powershell_native_installer_supports_persistent_docker_lifecycle`,
and restores the recorded
registry value only when it actually changed, so a passing run performs
zero registry writes and a
  regressed run cannot leave the contributor's PATH polluted.

The scope allow-list tests added by #2985 (`_ENSURE_PATH_SCOPE_HARNESS`,
`test_path_scope_accepts_process_case_insensitively`,
`test_path_scope_rejects_machine_and_invalid_values`)
are untouched.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

```text
$ uv run pytest tests/test_install/test_native_installers.py -q
platform win32 -- Python 3.13.11, pytest-9.0.3, pluggy-1.6.0
collected 5 items

tests\test_install\test_native_installers.py s....                       [100%]

======================== 4 passed, 1 skipped in 23.59s ========================

$ uv run ruff check .
All checks passed!

$ uv run ruff format --check tests/test_install/test_native_installers.py
1 file already formatted

$ uv run mypy headroom --ignore-missing-imports
Success: no issues found in 521 source files
```

The strengthened assertion was proven to detect a regression by
temporarily neutralising the scope
override in `scripts/install.ps1` (`if ($false -and
$env:HEADROOM_INSTALL_PATH_SCOPE)`), so
`Ensure-PathEntry` writes the `User` scope unconditionally again:

```text
$ uv run pytest tests/test_install/test_native_installers.py -q -k does_not_leak_into_user_path
tests\test_install\test_native_installers.py:638: in test_powershell_installer_does_not_leak_into_user_path
    assert str(home) not in (after[0] if after else ""), (
E   AssertionError: installer leaked the throwaway install dir into the real User PATH:
E     C:\Users\<user>\AppData\Local\Temp\pytest-of-<user>\pytest-154\test_powershell_installer_does0\home

======================= 1 failed, 4 deselected in 2.72s =======================
```

## Real Behavior Proof

- Environment: Windows 11 Pro 10.0.26200, PowerShell 7, Python 3.13.11,
pytest 9.0.3, headroom at
  `main` (`a6ab359a`), provider Anthropic
- Exact command / steps: recorded the raw `HKCU\Environment` `Path`
value with
`python -c "import winreg; ...QueryValueEx(k,'Path')"`, capturing its
registry kind, entry count
and a SHA-256 of the value; ran the full installer test file on the
patched tree; re-read the
registry; then neutralised the scope override in `scripts/install.ps1`
as shown above, re-ran the
single leak test, and re-read the registry a third time to confirm the
failure path restored it.
- Observed result: baseline `kind 1 entries 21 sha256 683ee646a95b8a28`.
After the passing run the
value was identical (`kind 1 entries 21 sha256 683ee646a95b8a28`), so a
passing run writes nothing.
With the override neutralised the test failed as quoted above and the
registry read afterwards was
again byte-identical to the recorded backup (compared as an exact
`{value, kind}` match, `True`),
confirming the `finally` restore. After reverting `scripts/install.ps1`,
the full file is back to
  4 passed, 1 skipped with the registry still unchanged.
- Not tested: non-Windows hosts (the changed test is Windows-only and
already skipped elsewhere;
`scripts/install.sh` is untouched), elevated/admin installs, and the
`Machine` scope, which
  `Ensure-PathEntry` rejects outright.

One open question this change is positioned to catch but does not
resolve: on this host the
`HKCU\Environment` `Path` value is `REG_SZ` (kind `1`), not
`REG_EXPAND_SZ`. A real install persists
through `[Environment]::SetEnvironmentVariable(..., 'User')`, which is
the API class known to rewrite
that value, so it is possible that a production install silently
downgrades an expandable PATH and
freezes `%USERPROFILE%`-style entries. I have not verified whether
headroom's installer caused it on
this machine or whether the value was always `REG_SZ`, and this PR
deliberately does not chase it.
Happy to open a separate issue if that is worth investigating.

## Runtime Rollout Safety

- Rollout-managed feature(s): none (test-only change)
- Minimum rollout channel: n/a
- Stable/default behavior changed: no; no production code path is
modified
- Kill switch / disable path: n/a
- Unsafe override required: no
- Qualification impact: none
- Rollback path: revert this commit; the test returns to the entry-count
comparison

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 20:21:03 -07:00
Abhay Singh
ddd9f76729
fix(install): stop the PowerShell installer leaking temp dirs into the real user PATH (#2985)
## Description

`scripts/install.ps1` persists the install directory to the user's PATH
through `Ensure-PathEntry`, which calls
`[Environment]::SetEnvironmentVariable('Path', ..., 'User')`. That value
lives in the `HKCU\Environment` registry key, so it is **not** scoped by
a `HOME` / `USERPROFILE` override.


`tests/test_install/test_native_installers.py::test_powershell_native_installer_supports_persistent_docker_lifecycle`
runs that real installer against a `tmp_path` fake home. Every run
therefore prepended the test's throwaway shim directory to the
developer's actual, persistent user PATH -- and it stayed there after
the test finished. The entries accumulate one per run, ahead of the real
install dir; and since the installer also drops
`headroom.ps1`/`headroom.cmd` into that dir, `headroom` in a fresh shell
could then resolve to a leftover wrapper from a deleted temp directory
(#2970).

## Fix

Make the persistence scope configurable via
`HEADROOM_INSTALL_PATH_SCOPE`, defaulting to `'User'` so production
behavior is unchanged:

```powershell
$scope = if ($env:HEADROOM_INSTALL_PATH_SCOPE) { $env:HEADROOM_INSTALL_PATH_SCOPE } else { 'User' }
$currentPath = [Environment]::GetEnvironmentVariable('Path', $scope)
...
[Environment]::SetEnvironmentVariable('Path', ($newPath -join ';'), $scope)
```

The installer tests (`_build_env`) set
`HEADROOM_INSTALL_PATH_SCOPE=Process`, so the PATH update stays in the
spawned PowerShell process (discarded when it exits) instead of writing
to the registry.

Fixes #2970

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `scripts/install.ps1` (`Ensure-PathEntry`): read/write the PATH via
`$env:HEADROOM_INSTALL_PATH_SCOPE` (default `'User'`).
- `tests/test_install/test_native_installers.py`: `_build_env` sets
`HEADROOM_INSTALL_PATH_SCOPE=Process` for every installer invocation;
add a Windows-only
`test_powershell_installer_does_not_leak_into_user_path` asserting the
real User PATH entry count is unchanged across an installer run.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] New test added

### Test Output

```text
tests/test_install/test_native_installers.py -k does_not_leak_into_user_path  1 passed
# uvx ruff@0.15.22 check tests/test_install/test_native_installers.py -> All checks passed!
```

## Real Behavior Proof

- Environment: Windows 11, Windows PowerShell 5.1, Python 3.12.11,
project venv, pytest 9.1.1, ruff 0.15.22 via uvx.
- Exact command / steps: recorded the real user PATH entry count
(`([Environment]::GetEnvironmentVariable('Path','User') -split
';').Count` = 27), ran the PowerShell installer test with the fix, then
re-read the count: still 27 -- no leak. The new
`test_powershell_installer_does_not_leak_into_user_path` formalizes this
(before == after).
- Observed result: running the installer test suite no longer mutates
the developer's persistent user PATH; production installs still persist
to `'User'` as before.
- Not tested: the sibling
`test_powershell_native_installer_supports_persistent_docker_lifecycle`
fails on my Windows host on an unrelated `trusted_cidrs`
dashboard-gateway assertion (it fails identically on `main` without this
change, and the whole PowerShell suite is skipped on the Linux CI
runners). This PR does not touch that path.

## Runtime Rollout Safety

- Rollout-managed feature(s): none. This is the native PowerShell
installer script, not a rollout-channel-gated runtime feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: no. Production installs still persist
PATH to the `User` scope exactly as before; the new
`HEADROOM_INSTALL_PATH_SCOPE` override defaults to `User` and is used
only by the test suite to avoid mutating the developer's persistent
PATH.
- Kill switch / disable path: leave `HEADROOM_INSTALL_PATH_SCOPE` unset
(the default) for the normal `User` behavior.
- Unsafe override required: no.
- Qualification impact: none. Installer-only; no proxy runtime path is
touched.
- Rollback path: revert this PR; the installer returns to writing the
`User` PATH unconditionally.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [x] New and existing unit tests pass locally with my changes
- [x] I did **not** edit `CHANGELOG.md`: it is generated by
release-please from my Conventional Commit PR title

## Additional Notes

The scope override defaults to `'User'`, so nothing changes for real
installs. It doubles as an escape hatch for any environment (CI images,
ephemeral containers) that must not touch the persistent user PATH.

---------

Co-authored-by: JD Davis <mxjerrett@gmail.com>
2026-08-16 15:05:04 -07:00
Suliman Abdulrazzaq
e044139001
fix(install): trust Docker bridge for dashboard metadata
## Summary

Closes #2909.

The `persistent-docker` installer now discovers Docker's default bridge
gateway and passes the exact `/32` gateway CIDR to the proxy's dashboard
metadata allowlist when no explicit
`HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS` value is configured.
This keeps the existing metadata gate intact while allowing the
first-party loopback-published container to see its own Recent Requests
and Per-Project Savings data. Explicit user configuration continues to
take precedence.

Both native wrappers (POSIX and PowerShell) use the same behavior, and
installer integration coverage verifies the generated Docker command.

## Validation

- `python -m pytest tests/test_install/test_native_installers.py -q -k
bash` (1 skipped on Windows because Bash is unavailable)
- PowerShell wrapper smoke test with the repository fake Docker shim:
verified `docker network inspect bridge` is called and
`HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS=172.17.0.1/32` is passed
to `docker run`
- Explicit allowlist smoke test: verified an existing
`HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS` is preserved without
adding a discovered default
- `git diff --check`

## Real behavior proof

Setup tested: Windows 11 host, PowerShell wrapper, repository fake
Docker shim (Docker CLI is not installed in this environment).

Exact command: `headroom.ps1 install apply --profile smoke --port 18999
--image fake/headroom:test`.

Observed result: the generated Docker invocation included `docker
network inspect bridge --format ...` and `--env
HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS=172.17.0.1/32`, and the
installer completed successfully.

Not tested: a live Docker daemon/dashboard request on this host.

---------

Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
2026-08-11 14:25:29 -07:00
chopratejas
967b0db439 fix: B1 — retire ICM, RollingWindow, scoring, relevance + dependents
Phase B step 1 of the live-zone-only realignment. Removes ~10K LOC of
"drop messages from history" machinery that became unreachable after
PR-A1 made `/v1/messages` a passthrough on the proxy. Live-zone-only
compression (PR-B2..B7) operates on content blocks within messages;
message-list mutation no longer happens in the pipeline.

Python deletes:
- headroom/transforms/intelligent_context.py (1077 LOC)
- headroom/transforms/rolling_window.py (395 LOC)
- headroom/transforms/progressive_summarizer.py (508 LOC)
- headroom/transforms/scoring.py (459 LOC)
- headroom/transforms/tool_crusher.py (338 LOC)
- 5 corresponding tests/test_transforms/* and tests/test_proxy_intelligent_context.py

Rust deletes:
- crates/headroom-core/src/context/* (manager, config, workspace,
  candidate, ccr_drop, strategy/, mod) + safety.rs replaced
- crates/headroom-core/src/scoring/* (mod, score, scorer, traits, weights)
- MessageScorerComparator from crates/headroom-parity (PR #338/#343
  becomes deletable; sunk cost stays sunk)
- 13 message_scorer fixtures + record_message_scorer.py

Rust adds (move + rewrite):
- crates/headroom-core/src/transforms/safety.rs — `tool_pair_indices`
  preserves the OpenAI/Anthropic tool_use ↔ tool_result pairing rule
  the live-zone dispatcher (PR-B2) needs. No IcmConfig dependency.

Surface refactors:
- HeadroomConfig: drop `tool_crusher`, `rolling_window`,
  `intelligent_context` fields; hoist `output_buffer_tokens` to top
  level (used by client.py).
- ProxyConfig: drop `intelligent_context*` fields.
- `headroom wrap` proxy server: retire IntelligentContextManager
  and RollingWindow imports + branch; pipeline is CacheAligner →
  ContentRouter (smart_routing) or CacheAligner → SmartCrusher
  (legacy).
- CLI: drop `--no-intelligent-context`, `--no-intelligent-scoring`,
  `--no-compress-first` flags.
- LangChain memory integration: rename `_apply_rolling_window` →
  `_apply_compression`, drop RollingWindowConfig dep. Threshold is
  now advisory — B6 will rework the contract.
- TransformPipeline.create_pipeline now takes only cache_aligner_config.
- headroom/__init__.py + headroom/transforms/__init__.py: strip
  exports of deleted symbols.

Bug fixes uncovered by full pytest sweep:
- providers/copilot/wrap.py: `environ or os.environ` collapsed
  empty-dict to falsy → callers passing `environ={}` accidentally
  pulled from os.environ. Use `environ if environ is not None else
  os.environ`.

Test correctness fixes:
- _DummyAnthropicHandler._retry_request gains **_kwargs to match
  the real handler signature post-A8.
- test_ws_http_fallback extracts JSON from `content=` (post-A3
  byte-faithful) rather than the obsolete `json=` kwarg.
- test_ccr_response_handler_extra fixture joins SSE events with
  `\n\n` per spec (post-A8 byte-buffer parser requirement).
- test_proxy_responses_phase_preservation: capture via direct
  handler attached to the named logger, so the assertion is
  order-independent (proxy `_setup_file_logging` flips
  `headroom.propagate=False` once any earlier test triggers it).
- conftest.py autouse fixture resets `headroom.propagate=True`
  before each test as a defensive measure for the same pollution.
- test_wrap_copilot_translated_backend_still_requires_byok:
  monkeypatch.delenv every provider key so the BYOK error
  actually fires.
- test_native_installers: skip when system bash < 4.3 (macOS ships 3.2).
- TestGeminiEmbedContent / TestGeminiBatchEmbedContents:
  pytest.mark.skip — proxy currently has no :embedContent route;
  feature gap, not regression.

Acceptance:
- cargo build --workspace + cargo clippy + cargo fmt --check: green.
- cargo test --workspace --exclude headroom-py: 777 passed.
- pytest: 4892 passed, 240 skipped, 0 failed.
- git grep returns only intentional comments referencing the deletion.

Per-PR-B1 plan: REALIGNMENT/04-phase-B-live-zone.md.
2026-05-02 12:23:17 -07:00
JerrettDavis
4a87753713 feat(docker): forward HEADROOM_WORKSPACE_DIR and HEADROOM_CONFIG_DIR into containers
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-16 19:19:25 -05:00
JerrettDavis
865bef2216 fix: harden persistent install wrappers and review gaps
Align Docker-native wrapper help and runtime behavior with the Python install contract, including persistent deployment metadata, baked install-image defaults, and explicit unsupported wrap targets.

Harden the Python persistent-install path with profile validation, safer provider-scope handling, Windows environment restoration, runtime parity improvements, and rollback-safe apply/update behavior.

Update README, Docker install docs, CI, and focused regressions to cover the Windows BOM failure, wrapper parity, compose coverage, and Docker-native wrap behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-11 18:03:21 -05:00
JerrettDavis
b325a06aae feat: harden persistent install wrappers
Tighten Docker-native bash and PowerShell wrapper validation for wrap and proxy flows, pin the bash wrapper to the install-time interpreter, clean up failed persistent container starts, and extend docs, CI, e2e, and native installer coverage for persistent Docker installs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-11 15:56:18 -05:00