security: validate version_files paths against traversal (Sprint 5 security audit finding) #181

Closed
opened 2026-06-27 12:11:13 +02:00 by quartermaster · 0 comments

Audit finding

Sprint 5 security audit (#156) flagged a low-medium severity path traversal vector in scripts/manifest-check.sh:112-139:

The version_files: config field accepts arbitrary paths without ../ rejection. A malicious config like version_files: [../../../../etc/passwd] would be read + text-parsed by manifest-check.sh, potentially leaking file contents.

Severity assessment

LOW-MEDIUM:

  • Only file CONTENT extraction, no execution
  • Affects only repos where the operator has authored a malicious config
  • PR review would catch a ../-bearing path before merge
  • Information disclosure scope is bounded to the runner's filesystem accessible by the workflow user

The realistic exploit surface: a malicious PR that adds version_files: [../../some/secret] to release-toolkit.yml, which gets merged and the next cut leaks the file contents to the rolling PR body or release notes.

Mitigations

  1. Reject paths containing ../ in config.sh's version_files parser
  2. Reject absolute paths (/etc/passwd)
  3. Bound to the consumer repo root (resolve + check against git rev-parse --show-toplevel)

Plus bats coverage for the new rejection paths.

Composition

  • Sister of the sed-injection tracker (other Sprint 5 security finding)
  • Both should ship together as a single "audit-finding-remediation" PR for cleaner cycle accounting

What this PR will NOT do

  • Will NOT change version_files semantics for legitimate paths
  • Will NOT block paths like subdir/VERSION (those are valid within the consumer repo)
  • Will NOT add a sandbox for manifest-check.sh (path validation is sufficient)

Refs

  • Sprint 5 #156 security audit
  • scripts/manifest-check.sh lines 112-139 (the audit finding's location)
  • scripts/lib/config.sh (the validation site)
## Audit finding Sprint 5 security audit (#156) flagged a low-medium severity path traversal vector in `scripts/manifest-check.sh:112-139`: The `version_files:` config field accepts arbitrary paths without `../` rejection. A malicious config like `version_files: [../../../../etc/passwd]` would be read + text-parsed by `manifest-check.sh`, potentially leaking file contents. ## Severity assessment LOW-MEDIUM: - Only file CONTENT extraction, no execution - Affects only repos where the operator has authored a malicious config - PR review would catch a `../`-bearing path before merge - Information disclosure scope is bounded to the runner's filesystem accessible by the workflow user The realistic exploit surface: a malicious PR that adds `version_files: [../../some/secret]` to `release-toolkit.yml`, which gets merged and the next cut leaks the file contents to the rolling PR body or release notes. ## Mitigations 1. **Reject paths containing `../`** in `config.sh`'s `version_files` parser 2. **Reject absolute paths** (`/etc/passwd`) 3. **Bound to the consumer repo root** (resolve + check against `git rev-parse --show-toplevel`) Plus bats coverage for the new rejection paths. ## Composition - Sister of the sed-injection tracker (other Sprint 5 security finding) - Both should ship together as a single "audit-finding-remediation" PR for cleaner cycle accounting ## What this PR will NOT do - Will NOT change `version_files` semantics for legitimate paths - Will NOT block paths like `subdir/VERSION` (those are valid within the consumer repo) - Will NOT add a sandbox for `manifest-check.sh` (path validation is sufficient) ## Refs - Sprint 5 #156 security audit - `scripts/manifest-check.sh` lines 112-139 (the audit finding's location) - `scripts/lib/config.sh` (the validation site)
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/release-toolkit#181
No description provided.