Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .githooks/pre-push
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#!/usr/bin/env bash
# PIPELINE PARITY: this hook mirrors .github/workflows/ci.yml.
# When editing this file, verify it still matches the pipeline (and vice
# versa — pipeline edits REQUIRE a hook audit; see CLAUDE.md).
# PIPELINE PARITY: this hook mirrors the `check` job of
# .github/workflows/ci.yml. When editing this file, verify it still matches
# the pipeline (and vice versa — pipeline edits REQUIRE a hook audit; see
# CLAUDE.md). The CI `coverage` job (just cov-ci) is intentionally NOT run
# pre-push: full llvm-cov instrumentation is too slow for every push, so the
# coverage floor is enforced in CI only. Run `just cov-ci` locally before a PR.
set -euo pipefail

echo "pre-push: cargo fmt --check"
Expand Down
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,22 @@ jobs:
run: |
cargo check -p modulex-cli --no-default-features --locked
cargo check -p modulex-mcp --no-default-features --locked

# COVERAGE GATE: mirrors `just cov-ci`. The floor (80% lines) is enforced
# here; keep it in sync with the justfile. Binary/pyo3 entrypoints are
# excluded (integration-tested, not unit-tested).
coverage:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
with:
components: llvm-tools-preview
- uses: Swatinem/rust-cache@v2
- name: Install cargo-llvm-cov
uses: taiki-e/install-action@cargo-llvm-cov
- name: Coverage (fail under the floor)
run: |
cargo llvm-cov --all-features --workspace \
--ignore-filename-regex '(main\.rs|modulex-py/src/lib\.rs)' \
--fail-under-lines 80
58 changes: 55 additions & 3 deletions crates/modulex-core/src/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,17 @@ impl CredentialRef {
if !out.success() {
return Err(CredentialError::CommandFailed(
cmd.clone(),
// stderr may carry diagnostics; stdout may carry the
// secret — only stderr is safe to surface.
out.stderr.trim().to_string(),
// NEITHER stream is safe to surface: a failing secret
// helper (`gh auth token`, `vault`, `pass`, custom
// wrappers) routinely echoes the very token it tried on
// BOTH stdout and stderr. This error is embedded into
// agent-visible step results (the credential-proxy
// premise is "the agent never sees a credential"), so
// we surface only the exit status, never the body.
match out.status {
Some(code) => format!("exited with status {code}"),
None => "did not exit normally".to_string(),
},
));
}
Ok(Secret::new(out.stdout.trim().to_string()))
Expand Down Expand Up @@ -219,6 +227,50 @@ mod tests {
assert!(denied.to_string().contains("vault"));
}

/// Adversarial regression (review #58): a failing `{cmd}` secret helper
/// routinely echoes the token it tried on its OWN stderr/stdout. That body
/// must NEVER reach the `CredentialError` — it is embedded into
/// agent-visible step results, the surface the credential proxy exists to
/// keep credential-free. Only the exit status is surfaced.
#[tokio::test]
async fn failing_cmd_helper_never_surfaces_its_stderr_body() {
use std::sync::Arc;

use agent_bridle_core::{Caveats, Scope};

use crate::exec::test_support::{gate_with, MockSpawner};

// The helper exits non-zero and prints the token it was resolving.
let leaked = "ghp_SECRET_ON_FAILURE_STDERR";
let spawner = Arc::new(MockSpawner::with_outputs(vec![MockSpawner::fail(
&format!("error: token {leaked} is expired, re-auth required"),
1,
)]));
let granted = Caveats {
exec: Scope::only(["gh".to_string()]),
..Caveats::top()
};
let gate = gate_with(&granted, spawner);

let err = CredentialRef::Cmd {
cmd: "gh auth token".into(),
}
.resolve(&gate)
.await
.unwrap_err();

let rendered = err.to_string();
assert!(
!rendered.contains(leaked),
"credential helper stderr leaked into the error: {rendered}"
);
// It still tells the operator what happened — just the status, no body.
assert!(
rendered.contains("status 1"),
"the failure should surface the exit status: {rendered}"
);
}

// The "Secret is not serializable" guarantee is enforced by the
// `compile_fail` doctest on the `Secret` type itself.
}
2 changes: 1 addition & 1 deletion crates/modulex-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub use exec::{ExecGate, ExecOutput, ExecRequest, Spawner, TokioSpawner};
pub use registry::StepRegistry;
pub use report::{RepoResult, Report, StepResult};
pub use step::{RunContext, StepHandler};
pub use store::Store;
pub use store::{McpServer, Store};

// Re-export the leash vocabulary so embedders don't need a direct
// agent-bridle-core dependency to construct grants.
Expand Down
Loading
Loading