fix(policy): keep wildcard allowlist entries literal in policy_evaluate (set -f)#52
Open
Pyronewbic wants to merge 1 commit into
Open
fix(policy): keep wildcard allowlist entries literal in policy_evaluate (set -f)#52Pyronewbic wants to merge 1 commit into
Pyronewbic wants to merge 1 commit into
Conversation
…te (set -f) policy_evaluate iterated the merged allowlist with `for h in $merged` (and the _PEVAL_EFFECTIVE / ALLOW_IPS loops) without set -f, so a documented glob-leading wildcard like *.s3.amazonaws.com was pathname-expanded against the invocation $PWD. apply_policy assigns SLUICE_ALLOW_DOMAINS from _PEVAL_EFFECTIVE, so this corrupted the ENFORCED egress allowlist on the run/build/shell path: a legit wildcard silently dropped when nothing matched, or got replaced by a sandbox-planted matching filename. Wrap the three split loops in one set -f span (matching the file's own discipline elsewhere). The case-based deny/allow/laundering matchers still glob-MATCH under set -f (set -f disables pathname expansion only), so policy refusals are unaffected. Also set -f the display-only loops (doctor/ls SLUICE_STATE_DIRS/ENV, cmd_ls --json ports). verify-policy-unit.bats #22 (deterministic, two planted files) guards it, proven RED against old; docs/policy.md notes the literal-wildcard / CWD-independence.
Pyronewbic
force-pushed
the
fix/policy-allowlist-noglob
branch
from
3b52326 to
7b071e2
Compare
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
1 participant
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.
Closes the bug-hunt major:
policy_evaluateglob-corrupted the enforced egress allowlist.The bug
policy_evaluateiterated the merged allowlist withfor h in $merged(and the_PEVAL_EFFECTIVE/ALLOW_IPSloops) withoutset -f, under the launcher's default glob-on. A documented glob-leading wildcard like*.s3.amazonaws.comwas pathname-expanded against the invocation$PWD. Sinceapply_policyassignsSLUICE_ALLOW_DOMAINSfrom_PEVAL_EFFECTIVE, this corrupted the enforced allowlist on the run/build/shell path:The same file already uses
set -ffor exactly this everywhere else (its own comment says "keep a wildcard like*.s3.amazonaws.comliteral").Fix
Wrap the three split loops in one
set -fspan. The case-based deny/allow/laundering matchers still glob-MATCH underset -f(it disables pathname expansion only), so policy refusals are unaffected -- the review fuzzed this (a.githubusercontent.comallow still covers adeny gist.githubusercontent.com; the laundering note still fires). Alsoset -f'd the display-only loops (doctor/lsSLUICE_STATE_DIRS/ENV,cmd_ls --jsonports).Verification
verify-policy-unit.batsTest restructure: cost-lane Makefile/CI split, box-harness helper, coverage gaps #22 -- deterministic (two planted*.s3.amazonaws.comfiles in CWD), proven RED against oldbin/sluice(old yields the filenames; fixed yields the literal wildcard).set -f;doctorshows the literal wildcard + the laundering note.build-checkin sync,lintclean,test-unit179/179.docs/policy.mdnotes literal-wildcard / CWD-independence; no THREAT_MODEL change (correctness fix, no stated guarantee altered).