fix(ci): the tool gate needs node, and an inert gate must say so
The first push of the PreToolUse gate failed CI, and the reason is a property of the gate worth fixing rather than a CI quirk. The hook parses its JSON payload with `node` — no jq in the runtime image, and node is guaranteed there because Claude Code is a node program. CI runs `cargo test --workspace` inside `rust:1.96-slim`, which has no node. The extraction returned nothing, the gate allowed everything, and the two "blocks" tests failed. That is correct behaviour with a dangerous appearance. A gate that cannot read its input must not block the phase — failing closed on a parse error denies every tool call, which is what an earlier `case`-syntax bug did. But allowing silently makes an INERT gate indistinguishable from one that simply matched nothing, which is this codebase's recurring defect exactly. So the gate now records `inert` when node is absent, still allowing, and a test pins both halves: exit 0, and the marker written. The host can check for that file rather than infer a working gate from an absence of denials. CI installs nodejs so the shell tests exercise the gate instead of its inert path. Verified in a rust:1.96-slim container: without node the force-push payload returns 0, with node it returns 2. Also confirmed the generated script behaves under dash — Linux /bin/sh — not only under macOS sh. An earlier apparent dash failure was invalid JSON in the probe command, not the gate. Full workspace suite green: 106 binaries. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b653dbfe72
commit
128b423205
@@ -89,7 +89,13 @@ jobs:
|
|||||||
rust:1.96-slim \
|
rust:1.96-slim \
|
||||||
sh -c 'set -e
|
sh -c 'set -e
|
||||||
apt-get update -qq
|
apt-get update -qq
|
||||||
apt-get install -y -qq pkg-config libssl-dev cmake git >/dev/null
|
# nodejs: cm-api's vm_tool_gate shell tests EXECUTE the generated
|
||||||
|
# PreToolUse hook, which parses its JSON payload with node (no jq
|
||||||
|
# in the runtime image; node is guaranteed there because Claude
|
||||||
|
# Code is a node program). Without it the hook takes its
|
||||||
|
# allow-and-record-inert path and the two "blocks" tests fail —
|
||||||
|
# which is how this was found, on the first push that carried them.
|
||||||
|
apt-get install -y -qq pkg-config libssl-dev cmake git nodejs >/dev/null
|
||||||
git config --global url."https://oauth2:[email protected]/".insteadOf "https://git.redclaw.dev/"
|
git config --global url."https://oauth2:[email protected]/".insteadOf "https://git.redclaw.dev/"
|
||||||
cargo test --workspace'
|
cargo test --workspace'
|
||||||
|
|
||||||
|
|||||||
@@ -45,6 +45,16 @@ pub const GUEST_DIR: &str = "/root/toolgate";
|
|||||||
/// The file the gate appends a line to for every denial.
|
/// The file the gate appends a line to for every denial.
|
||||||
pub const DENIED_FILE: &str = "denied.jsonl";
|
pub const DENIED_FILE: &str = "denied.jsonl";
|
||||||
|
|
||||||
|
/// Written when the gate is installed but cannot function.
|
||||||
|
///
|
||||||
|
/// The gate needs `node` to read the hook payload. Without it the extraction
|
||||||
|
/// returns nothing and every call is allowed — correct behaviour (never fail
|
||||||
|
/// closed) with a dangerous appearance: an inert gate and a gate that simply
|
||||||
|
/// matched nothing produce identical output. This marker is the difference,
|
||||||
|
/// and the host can check for it. Found because CI's `rust:1.96-slim` has no
|
||||||
|
/// node and the gate passed everything there.
|
||||||
|
pub const INERT_FILE: &str = "inert";
|
||||||
|
|
||||||
/// How a rule's needle is matched.
|
/// How a rule's needle is matched.
|
||||||
#[derive(PartialEq, Eq, Clone, Copy)]
|
#[derive(PartialEq, Eq, Clone, Copy)]
|
||||||
enum Match {
|
enum Match {
|
||||||
@@ -218,6 +228,12 @@ pub fn hook_script(dir: &str) -> String {
|
|||||||
payload=$(cat)\n\
|
payload=$(cat)\n\
|
||||||
# Tool name on line 1, command on line 2. Anything unparseable prints\n\
|
# Tool name on line 1, command on line 2. Anything unparseable prints\n\
|
||||||
# nothing and the gate allows — never fail closed here.\n\
|
# nothing and the gate allows — never fail closed here.\n\
|
||||||
|
if ! command -v node >/dev/null 2>&1; then\n\
|
||||||
|
\x20 # Allow, but SAY SO. A gate that cannot read its input must not\n\
|
||||||
|
\x20 # block the phase, and must not look like one that found nothing.\n\
|
||||||
|
\x20 echo 'no node: tool gate is inert' >> {dir}/{inert} 2>/dev/null\n\
|
||||||
|
\x20 exit 0\n\
|
||||||
|
fi\n\
|
||||||
info=$(printf '%s' \"$payload\" | node -e '{extract}' 2>/dev/null)\n\
|
info=$(printf '%s' \"$payload\" | node -e '{extract}' 2>/dev/null)\n\
|
||||||
tool=$(printf '%s\\n' \"$info\" | sed -n 1p)\n\
|
tool=$(printf '%s\\n' \"$info\" | sed -n 1p)\n\
|
||||||
cmd=$(printf '%s\\n' \"$info\" | sed -n 2p)\n\
|
cmd=$(printf '%s\\n' \"$info\" | sed -n 2p)\n\
|
||||||
@@ -240,6 +256,7 @@ pub fn hook_script(dir: &str) -> String {
|
|||||||
# Nothing matched. Exit 0 ALLOWS the call.\n\
|
# Nothing matched. Exit 0 ALLOWS the call.\n\
|
||||||
exit 0\n",
|
exit 0\n",
|
||||||
extract = NODE_EXTRACT,
|
extract = NODE_EXTRACT,
|
||||||
|
inert = INERT_FILE,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -411,6 +428,57 @@ mod shell_tests {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Without `node` the gate cannot read its input. It must ALLOW — blocking
|
||||||
|
/// the phase because a parser is missing is the worse failure — and it must
|
||||||
|
/// leave evidence, because an inert gate otherwise looks exactly like one
|
||||||
|
/// that found nothing. CI's rust:1.96-slim has no node, which is how this
|
||||||
|
/// was found.
|
||||||
|
#[test]
|
||||||
|
fn without_node_the_gate_allows_but_records_that_it_is_inert() {
|
||||||
|
static N: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(9000);
|
||||||
|
let seq = N.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
|
||||||
|
let dir = std::env::temp_dir().join(format!("cm-gate-nonode-{}-{seq}", std::process::id()));
|
||||||
|
std::fs::create_dir_all(&dir).unwrap();
|
||||||
|
let script = dir.join("tool-gate.sh");
|
||||||
|
std::fs::write(&script, hook_script(&dir.to_string_lossy())).unwrap();
|
||||||
|
|
||||||
|
// An absolute shell with a PATH that contains nothing: `node` is
|
||||||
|
// unfindable, and `sh` is still spawnable. An empty PATH would fail to
|
||||||
|
// find the shell itself, which tests nothing.
|
||||||
|
let empty = dir.join("emptybin");
|
||||||
|
std::fs::create_dir_all(&empty).unwrap();
|
||||||
|
let mut child = Command::new("/bin/sh")
|
||||||
|
.arg(&script)
|
||||||
|
.env("PATH", &empty)
|
||||||
|
.stdin(Stdio::piped())
|
||||||
|
.stdout(Stdio::piped())
|
||||||
|
.stderr(Stdio::piped())
|
||||||
|
.spawn()
|
||||||
|
.expect("spawn sh");
|
||||||
|
child
|
||||||
|
.stdin
|
||||||
|
.as_mut()
|
||||||
|
.unwrap()
|
||||||
|
.write_all(
|
||||||
|
br#"{"tool_name":"Bash","tool_input":{"command":"git push --force origin main"}}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
let out = child.wait_with_output().expect("wait");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
out.status.code(),
|
||||||
|
Some(0),
|
||||||
|
"a gate that cannot parse must not block the phase"
|
||||||
|
);
|
||||||
|
let marker = dir.join(INERT_FILE);
|
||||||
|
assert!(
|
||||||
|
marker.exists(),
|
||||||
|
"an inert gate must leave evidence — otherwise it is indistinguishable \
|
||||||
|
from a gate that matched nothing"
|
||||||
|
);
|
||||||
|
let _ = std::fs::remove_dir_all(&dir);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn the_shell_blocks_a_force_push_with_exit_2_and_a_reason() {
|
fn the_shell_blocks_a_force_push_with_exit_2_and_a_reason() {
|
||||||
let payload = r#"{"tool_name":"Bash","tool_input":{"command":"git push --force origin main"}}"#;
|
let payload = r#"{"tool_name":"Bash","tool_input":{"command":"git push --force origin main"}}"#;
|
||||||
|
|||||||
Reference in New Issue
Block a user