mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
7 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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.
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |