fix(trust): eliminate TOCTOU race in filter loading; harden CI trust bypass#2050
Open
rosschurchill wants to merge 2 commits into
Open
fix(trust): eliminate TOCTOU race in filter loading; harden CI trust bypass#2050rosschurchill wants to merge 2 commits into
rosschurchill wants to merge 2 commits into
Conversation
…rust bypass C-01 (TOCTOU): toml_filter.rs previously called check_trust() (which reads+hashes the file) then fs::read_to_string() a second time. A race-replacement between the two reads could inject arbitrary filter rules. Fix: read once into Vec<u8>, compute SHA-256 in-process, call new check_trust_from_hash() which never re-reads the file. CI bypass hardening (Chain A): removed bare CI=true from the allowed list for RTK_TRUST_PROJECT_FILTERS=1. Only verified platform variables (GITHUB_ACTIONS, GITLAB_CI, JENKINS_URL, BUILDKITE) now grant EnvOverride. CI=true is trivially settable in .envrc and was a local-bypass vector. Chain A risk gate: compile_filter() now calls validate_filter_safety() before adding a filter to the registry. Rejects on_empty="pass", head_lines=1, and match_command=".*"/".*+" — the three patterns identified in the Chain A exploit. Also gates all eprintln! sites in trust.rs (2) and toml_filter.rs (13) behind in_hook_mode() to fix C-07 in these files (C-07 helper landed in prior commit). Adds TOCTOU regression test and CI-bypass test in trust.rs test module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ore helper test_env_override_with_ci used generic CI=true which C-01 intentionally removed — update to use GITHUB_ACTIONS to match the new hardened policy. test_ci_bypass_requires_platform_ci_not_generic and test_env_override_with_ci both mutate the same env vars; add ENV_TEST_LOCK mutex to serialize them. Add check_trust_from_hash_with_store test helper so the TOCTOU test can verify hash comparison without reading from the real trust store. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two issues in the filter trust path:
TOCTOU race (CWE-367):
check_trust()reads and hashes the filter file, thenload()re-reads it viafs::read_to_string(). An attacker can swap a malicious file into the race window between the two reads. Fix: read once intoVec<u8>, compute SHA-256 in-process, pass the pre-computed hash to a newcheck_trust_from_hash()— the file is never re-read.CI trust bypass:
CI=trueis trivially settable in any.envrcor shell profile, making the env-var trust override reachable outside CI. Tightened to require a platform-specific var (GITHUB_ACTIONS,GITLAB_CI,JENKINS_URL, orBUILDKITE).Also adds a
validate_filter_safety()gate incompile_filter()that rejects obviously dangerous filter patterns (on_empty=pass,head_lines=1,match_command=.*).Free to use as-is.