security: Harden remote cache output hydration against undeclared paths - #2562
Conversation
There was a problem hiding this comment.
Pull request overview
Hardens moon_task_runner remote/local cache output hydration by normalizing and validating output paths before writing to disk, preventing compromised cache manifests from writing outside the workspace or outside declared outputs.
Changes:
- Added workspace-root and “declared output” path resolution in
OutputHydraterbefore writing hydrated files/symlinks. - Introduced a new task-runner error for undeclared hydrated outputs and added regression tests for traversal/absolute/undeclared hydration attempts.
- Refactored
OutputArchiver/OutputHydraterconstruction behindnew(...)constructors and documented the security fix in the changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/task-runner/src/output_hydrater.rs | Enforces path normalization + workspace/output-declaration checks before writing hydrated outputs and creating symlinks. |
| crates/task-runner/src/task_runner_error.rs | Adds a new error variant for rejecting undeclared hydrated outputs. |
| crates/task-runner/tests/output_hydrater_test.rs | Adds regression coverage for directory-output hydration and malicious cache manifest paths (traversal/absolute/undeclared). |
| crates/task-runner/tests/utils.rs | Updates test helpers to construct archiver/hydrater via new constructors. |
| crates/task-runner/src/output_archiver.rs | Introduces a constructor and makes fields private to centralize initialization. |
| crates/task-runner/src/task_runner.rs | Updates TaskRunner to construct archiver/hydrater via new(...). |
| crates/daemon-server/src/daemon_server.rs | Updates daemon archiving path to use OutputArchiver::new(...). |
| CHANGELOG.md | Adds an Unreleased security note describing the cache-hydration overwrite fix. |
Merging this PR will improve performance by 35.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | load_one |
40.8 ms | 29.5 ms | +38.15% |
| ⚡ | WallTime | load_all |
138.5 ms | 102.2 ms | +35.5% |
| ⚡ | WallTime | load_many |
57.3 ms | 42.8 ms | +33.87% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/verify-moon-vulnerability-fix (037227a) with master (01793e7)
Unable to generate the flame graphsThe performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists. |
Summary
Testing
cargo test -p moon_task_runner --test output_hydrater_test -- --nocapturecargo check -p moon_task_runnercargo clippy -p moon_task_runner --all-targets -- -D warnings