Skip to content

security: constrain standalone application payloads - #520

Merged
codeforester merged 4 commits into
mainfrom
security/510-20260917-security-define-an-explicit-application-payload-for-standalo
Sep 18, 2026
Merged

codeforester merged 4 commits into
mainfrom
security/510-20260917-security-define-an-explicit-application-payload-for-standalo

Conversation

@codeforester

Copy link
Copy Markdown
Collaborator

Summary

Replace recursive application-tree copying with a fixed runtime allowlist and explicit --include support for files under assets/ or config/. Reject symlink/special payload entries and destinations inside the application source tree, and document the distribution boundary.

Issue

Fixes #510

Validation

  • bats tests/vendor.bats — passed (7 tests).
  • bats --filter standalone tests/vendor.bats — passed (2 tests).
  • ShellCheck, shfmt -d -ln bash -i 4 -sr scripts/vendor, and git diff --check — passed.
  • Repository-wide shfmt contract is left to hosted CI because the installed local formatter version reports unrelated baseline formatting changes.

Docs Impact

Documents the default allowlist, explicit optional assets, safe path rules, and external output-directory requirement.

Security Notes

Default packaging no longer traverses arbitrary application files; local markers, repository metadata, tests, and stale output are excluded.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Multi-angle review of this PR (fixed runtime allowlist replacing recursive copy_tree, addresses #510). This is a security-boundary change so I pushed hard on it — four findings, ranked by severity; the third was confirmed by a live repro, not just static reading:

1. The VERSION/README.md "restore" copy late in standalone_bundle bypasses validate_payload_file entirely. Both files are members of the fixed payload_files allowlist and get validated once (symlink-per-component walk + -f check) and copied safely via copy_payload_files. But later in the same function, after copy_verified_bundle stamps the framework's own VERSION/README.md over the standalone root, the code re-copies the application's VERSION/README.md a second time via a raw cp -- (around lines 314-315) with no -L/symlink check at all. docs/vendor-workflow.md:48 explicitly documents "Included paths must be regular files with no symlink in any path component" as a guarantee of the payload mechanism — this second, unguarded path for two of those exact entries doesn't honor it. If either file becomes a symlink (e.g. via a concurrent build step) after the initial validation loop but before this later restore step — and substantial work happens in between (destination checks, mktemp, two copy_verified_bundle calls, write_lock) — the restore cp silently follows it and copies the target's contents into the shipped bundle.

2. TOCTOU gap between validate_payload_file's symlink check and copy_payload_files's actual cp, for every payload path. The old copy_tree validated and copied each file in the same loop iteration; the new design splits this into two temporally separated passes, with real work (the destination-existence check, mktemp -d) between them. copy_payload_files does not re-check -L before its cp -- (line ~91). A symlink swapped into the source tree in that window is silently followed — the exact class of traversal validate_payload_file was introduced to prevent, just moved outside its enforcement window.

3. standalone_destination_is_external's containment check is bypassable on case-insensitive filesystems (the macOS/APFS default) — confirmed by live repro. The guard compares canonicalized paths with a case-sensitive bash [[ ]] match ("$canonical" != "$source/"*), but bash's builtin pwd -P does not case-normalize the way the OS does on a case-insensitive volume. Live repro: created an application at .../CaseApp, then ran scripts/vendor standalone .../CaseApp .../framework-bundle .../CASEAPP/dist-inside — this exited 0 and created the bundle, even though .../CASEAPP/dist-inside and .../CaseApp/dist-inside are verifiably the same on-disk directory. The guard this PR specifically added is silently defeated whenever the destination path differs from the application path only in case.

4. The fixed six-file payload allowlist is hand-duplicated in two places inside this same file, and again against bin/base-bash's generator. payload_files (line ~248) is re-listed via a case pattern in the selection loop (~280-287) purely to distinguish "known-fixed" from "must be under assets/ or config/" — and the same six-name list is independently hardcoded a third time in bin/base-bash:622's base_launcher_init() (what base-bash init actually scaffolds). Adding a 7th default file to the generator requires updating all of this by hand; forgetting the case copy here rejects a legitimate default file with a misleading "must be explicitly selected under assets/ or config/" error. Given this repo already has lib/bash/file/lib_file.sh for file-safety primitives (and scripts/library-bundle independently reimplements a similar symlink-rejection check), the deeper fix for both this and findings 1-2 is likely to promote the symlink-walk/containment checks into a shared, tested lib/bash/file helper rather than three private reimplementations across the repo.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Addressed in commit 26f0aef: a single packaged payload manifest now feeds both project generation and standalone packaging. All required and optional payloads are staged through the same symlink checks and pre/post-copy digest verification; the unguarded README/VERSION recopy is gone, and destination containment uses filesystem identity. Vendor and launcher suites pass (45 tests), including forced leaf/parent symlink swaps and case-aliased containment.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Follow-up: hosted ShellCheck found an unused local in the new race-test fixture. Removed in c093973; ./tests/lint-warnings.sh now passes locally, and the refreshed hosted checks are queued.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Confirmed fixed under active adversarial re-testing, not just reading the diff — this was the security-sensitive one so I pushed hard on it:

  • Turned the application's VERSION/README.md into symlinks pointing outside the tree and re-ran standalone bundling: now correctly rejected (application payload path traverses a symlink), where before it silently leaked the symlink target's contents.
  • Built an independent PATH-shadowing cp interceptor to hit the TOCTOU window at the exact moment of copy (a tighter race than the PR's own test fixtures): now detected and rejected (application payload changed to a symlink while it was copied) rather than silently followed.
  • Re-ran the exact case-insensitive-filesystem repro (destination spelled in different case than the same physical source directory): now correctly rejected via filesystem-identity (-ef) checks instead of a case-sensitive string prefix compare.
  • The six-file payload allowlist is now a single source of truth (scripts/standalone-app-payloads.txt) read by both scripts/vendor and bin/base-bash's generator.

All four hold under adversarial testing.

@codeforester
codeforester merged commit 8de2056 into main Sep 18, 2026
10 of 11 checks passed
@codeforester
codeforester deleted the security/510-20260917-security-define-an-explicit-application-payload-for-standalo branch September 18, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: define an explicit application payload for standalone packaging

1 participant