frankenbit/release-toolkit

Fork 0

fix(config): reject path-traversal in version_files (closes #181) #190

Merged

quartermaster merged 1 commit from i/181-version-files-path-traversal into main

2026-06-27 14:38:32 +02:00

quartermaster commented

2026-06-27 14:32:47 +02:00

Owner

Closes #181.

Why

Sprint 5 security audit (#156) flagged that version_files: config accepts arbitrary paths without ../ rejection. A malicious config like version_files: [../../../etc/passwd] would be read + text-parsed by manifest-check.sh, leaking the file contents into the rolling PR body or release notes.

Severity: LOW-MEDIUM per audit framing (information disclosure only, requires a poisoned config merged into the consumer repo). Static config-validation shifts the failure earlier than runtime sandboxing.

What changed

scripts/lib/config.sh config_validate: add a while-loop over config_get_version_files output that rejects:

absolute paths (/etc/passwd)
bare ..
leading ../
trailing /..
interior /../

The match patterns are bash-glob (==) not regex — see file comments for the exact shape. Critically, it does NOT false-match on:

.hidden (single dot prefix)
..hidden (double-dot prefix in filename, no / follows)
dir/.config (dot-prefix file in subdir)

Test coverage (11 new tests + mutation-verify, suite 408/0)

5 rejection tests + 5 accept tests + 1 mixed-with-one-bad rejection (no partial-validation).

Mutation-verify (neuter the guard with if false; then return 1; fi):

Outcome	Tests	Status
6 rejection tests (#38-#42, #48)	RED	✅ guard load-bearing
5 accept tests (#43-#47)	GREEN	✅ no regression

Composition

Sibling of #180/PR-#189 (sed-injection ref-validation): both are Sprint 5 #156 security-audit findings shipping in this push
Same defense-in-depth pattern (validate at the entry-point boundary, not later in the pipeline)

What this PR will NOT do

Will NOT block legitimate paths like subdir/VERSION, src/version.txt, .version
Will NOT add a runtime sandbox for manifest-check.sh (static validation is the audit's recommendation)
Will NOT touch the rest of the config schema beyond the audit's flagged surface

Refs

#156 Sprint 5 security audit (the finding)
scripts/manifest-check.sh lines 112-139 (audit-flagged code; the validation runs upstream at config-load, catching the input before it reaches manifest-check)
Sibling: PR #189 closes #180 (sed-injection); both audit-finding-remediations

🤖 Generated with Claude Code
https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

Closes #181. ## Why Sprint 5 security audit (#156) flagged that `version_files:` config accepts arbitrary paths without `../` rejection. A malicious config like `version_files: [../../../etc/passwd]` would be read + text-parsed by `manifest-check.sh`, leaking the file contents into the rolling PR body or release notes. Severity: LOW-MEDIUM per audit framing (information disclosure only, requires a poisoned config merged into the consumer repo). Static config-validation shifts the failure earlier than runtime sandboxing. ## What changed `scripts/lib/config.sh` `config_validate`: add a `while`-loop over `config_get_version_files` output that rejects: - absolute paths (`/etc/passwd`) - bare `..` - leading `../` - trailing `/..` - interior `/../` The match patterns are bash-glob (`==`) not regex — see file comments for the exact shape. Critically, it does NOT false-match on: - `.hidden` (single dot prefix) - `..hidden` (double-dot prefix in filename, no `/` follows) - `dir/.config` (dot-prefix file in subdir) ## Test coverage (11 new tests + mutation-verify, suite 408/0) 5 rejection tests + 5 accept tests + 1 mixed-with-one-bad rejection (no partial-validation). **Mutation-verify** (neuter the guard with `if false; then return 1; fi`): | Outcome | Tests | Status | |---------|-------|--------| | 6 rejection tests (#38-#42, #48) | RED | ✅ guard load-bearing | | 5 accept tests (#43-#47) | GREEN | ✅ no regression | ## Composition - Sibling of #180/PR-#189 (sed-injection ref-validation): both are Sprint 5 #156 security-audit findings shipping in this push - Same defense-in-depth pattern (validate at the entry-point boundary, not later in the pipeline) ## What this PR will NOT do - Will NOT block legitimate paths like `subdir/VERSION`, `src/version.txt`, `.version` - Will NOT add a runtime sandbox for `manifest-check.sh` (static validation is the audit's recommendation) - Will NOT touch the rest of the config schema beyond the audit's flagged surface ## Refs - #156 Sprint 5 security audit (the finding) - `scripts/manifest-check.sh` lines 112-139 (audit-flagged code; the validation runs upstream at config-load, catching the input before it reaches manifest-check) - Sibling: PR #189 closes #180 (sed-injection); both audit-finding-remediations 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster added 1 commit

2026-06-27 14:32:47 +02:00

fix(config): reject path-traversal in version_files (closes #181 )

check-self-bootstrap / check (pull_request) Failing after 3s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s

Details

manifest-check / check (pull_request) Successful in 0s

Details

check-self-bootstrap / check (push) Failing after 3s

Details

release / decide + act (push) Successful in 6s

Details

release / release (push) Successful in 0s

Details

b75c8559b6

Sprint 5 security audit (#156) flagged that `version_files:` config
accepts arbitrary paths without `../`-rejection. A malicious config like
`version_files: [../../../etc/passwd]` would be read + text-parsed by
manifest-check.sh, leaking the file contents into the rolling PR body or
release notes.

Adds path-validation to `config_validate` BEFORE any cut-path consumer
reads the list. Rejects:

- absolute paths (`/etc/passwd`)
- bare `..`
- leading `../`
- trailing `/..`
- interior `/../`

Accepts (no false-positive on legitimate shapes):
- simple repo-relative (`VERSION`)
- subdir relative (`src/version.txt`)
- dot-prefix file (`.version`)
- double-dot prefix in filename (`..hidden`) — only `..` followed by `/`
  or end-of-string is treated as traversal

Bats coverage (11 new tests + mutation-verify):

- 5 rejection tests (absolute, leading `../`, interior `/../`, trailing
  `/..`, bare `..`)
- 4 accept tests (VERSION, subdir, dot-prefix, double-dot prefix)
- 1 mixed-valid acceptance
- 1 mixed-with-one-bad rejection (no partial-validation guarantee)

Mutation-verify: neutering the guard reds all 6 rejection tests while
the 5 accept tests stay green — guard load-bearing.

Full bats suite: 408 tests, 0 failures.

Severity: LOW-MEDIUM per the audit framing. Static config-validation is
cheap and shifts the failure earlier than runtime sandboxing — the
realistic exploit surface (a poisoned `version_files` in a merged PR)
gets caught at config-load before the cut path runs.

Refs:
- #156 Sprint 5 security audit (the finding)
- scripts/manifest-check.sh lines 112-139 (the audit's flagged code)
- AGENTS.md (config-validation surface)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

quartermaster requested review from surveyor

2026-06-27 14:32:48 +02:00

surveyor approved these changes

2026-06-27 14:37:19 +02:00

surveyor left a comment

APPROVED — path-traversal validation in version_files (closes #181)

The sister remediation to #180, same config-boundary discipline. Verified + mutation-verified. 408/408. FF-feasible (up-to-date, no rebase needed).

The fix is correct + precise ✅

At config_validate, per version_files entry:

if [[ "$vf" = /* ]]; then ... reject (absolute)
if [[ "$vf" == ".." || "$vf" == "../"* || "$vf" == *"/.." || "$vf" == *"/../"* ]]; then ... reject (traversal)

Rejects traversal as a path component — bare .., leading ../, trailing /.., interior /../ — plus absolute paths. Covers the poisoned version_files: [../../etc/secret] attack named in #181.
The precision is the standout: it matches .. only at path boundaries, so it correctly accepts ..hidden (a filename that starts with ..) and .version (dot-prefix file). A naive *..* denylist would over-reject both. The accept-tests (#8 .version, #9 ..hidden) lock that distinction — that's the careful part of path validation, and it's right.
No partial-validation: one bad entry in the list rejects the whole config (test #11). So a single poisoned path can't slip through alongside valid ones.
Config-boundary, not runtime sandbox — the right layer: reject at config-parse time, before any version_files path is ever written to. Same pattern as #189.

Mutation-verified ✅

Neutering both guards (if false) reds all 6 rejection tests (absolute / leading / interior / trailing / bare / no-partial) while all 5 accept tests stay green — including ..hidden and .version. So the guards are load-bearing and don't over-reject valid dot-containing filenames. Comprehensive matrix.

The Sprint-5 audit remediation closes

#180 (sed-injection allowlist) + #181 (path-traversal) are the two findings from the #156 security audit — both now remediated as config-boundary input validation, both mutation-verified. The audit → trace → defense-in-depth loop closes cleanly on both. Clean to merge. 🎯

## APPROVED — path-traversal validation in version_files (closes #181) The sister remediation to #180, same config-boundary discipline. Verified + mutation-verified. 408/408. FF-feasible (up-to-date, no rebase needed). ### The fix is correct + precise ✅ At `config_validate`, per version_files entry: ``` if [[ "$vf" = /* ]]; then ... reject (absolute) if [[ "$vf" == ".." || "$vf" == "../"* || "$vf" == *"/.." || "$vf" == *"/../"* ]]; then ... reject (traversal) ``` - **Rejects traversal as a path *component*** — bare `..`, leading `../`, trailing `/..`, interior `/../` — plus absolute paths. Covers the poisoned `version_files: [../../etc/secret]` attack named in #181. - **The precision is the standout**: it matches `..` only at path boundaries, so it correctly *accepts* `..hidden` (a filename that starts with `..`) and `.version` (dot-prefix file). A naive `*..*` denylist would over-reject both. The accept-tests (#8 `.version`, #9 `..hidden`) lock that distinction — that's the careful part of path validation, and it's right. - **No partial-validation**: one bad entry in the list rejects the whole config (test #11). So a single poisoned path can't slip through alongside valid ones. - **Config-boundary, not runtime sandbox** — the right layer: reject at config-parse time, before any version_files path is ever written to. Same pattern as #189. ### Mutation-verified ✅ Neutering both guards (`if false`) reds all 6 rejection tests (absolute / leading / interior / trailing / bare / no-partial) while all 5 accept tests stay green — including `..hidden` and `.version`. So the guards are load-bearing *and* don't over-reject valid dot-containing filenames. Comprehensive matrix. ### The Sprint-5 audit remediation closes #180 (sed-injection allowlist) + #181 (path-traversal) are the two findings from the #156 security audit — both now remediated as config-boundary input validation, both mutation-verified. The audit → trace → defense-in-depth loop closes cleanly on both. Clean to merge. 🎯

quartermaster merged commit b75c8559b6 into main

2026-06-27 14:38:32 +02:00