fix(rules): confine the raw path, not the redacted target (security C1)#89
Merged
Merged
Conversation
…rget A path value >256 chars (or secret-shaped) was redacted to "<redacted>" before ProtectedPathRule ran, so a padded traversal or a padded path resolving to a protected glob canonicalized to a harmless in-root string and PASSed while the executor forwarded the real path. The rule now confines the raw path from ctx.metadata['raw_arguments'] (falling back to action.target); the raw path is used for matching only and never enters action.target or any log field. Adds tests/unit/test_path_confinement_raw.py (padded-traversal BLOCK, padded blocked/sensitive globs, batch, redaction-preserved, no-regression).
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
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.
Slice
What this PR does
Closes a confirmed path-confinement bypass.
ProtectedPathRuleevaluated the redactedaction.target:normalizereplaces any string arg >256 chars (or secret-shaped) with the literal"<redacted>"before it becomesaction.target, so a padded../traversal (e.g.("../"*90)+"etc/passwd", 270 chars) — or a padded path that canonicalizes to a protected/sensitive glob — was confined as the harmless marker, returned PASS, and the executor forwarded the real path downstream. This defeated both theescapes_rootblock and the protected-path glob block.The rule now confines the raw path from
ctx.metadata['raw_arguments'](the same un-redacted args the secret and destructive-command rules already read), falling back toaction.target/metadata['raw_paths']when raw args are absent. The raw path is used for matching only — it never entersaction.target,action.metadata, or any log field (build_recordstill excludes metadata; the persistedtarget_path_classstays redacted).No import-boundary change: the key list is local to
engine/rules/paths.py;enginedoes not importproxy(lint-importsgreen).Tests added (run in CI)
tests/unit/test_path_confinement_raw.py(9 tests): padded-traversal → BLOCK; padded blocked-glob → BLOCK; padded sensitive-glob → AUTH; batch raw paths escalate to the worst member; raw path never leaks intoaction.targetor anybuild_recordfield; short.envstill BLOCKs; normal short path still PASSes; a ctx with noraw_argumentsfalls back toaction.target. The 4 bypass tests return PASS onmainand BLOCK/AUTH with this fix.Public-release safety (doberman-core only)
Security checklist
action.targetor the log record)protected_path_blocked/sensitive_path_access+ existing static explanations; no raw path in the explanation)Edge cases covered / Deviations from plan / Risks introduced
_RAW_PATH_KEYSis deliberately the file-path keys (path/file/filename/target), noturl/command— including those would make the rule canonicalize URLs/commands as paths and risk false positives; the file actionsProtectedPathRuleguards carry their path in these keys.raw_arguments.lint-importsclean.