ref:09ed84e155c2c96bfc94e0f2d289ee6afbea28eb

test: cover the runner's untested surface, and fix what the tests found (#58)

Coverage pass over `anvil-cli`, run against a written standard (`docs/TESTING.md`) rather than at the coverage number. ## Numbers | | before | after | |---|---|---| | Regions | 64.15% | **71.83%** | | Lines | 65.88% | **72.74%** | | Functions | 73.35% | **81.71%** | | Lib tests | 232 | **364** | Five modules were at literal 0%. | module | before | after | |---|---|---| | `runner/artifacts.rs` | 0% | 97.63% | | `runner/heartbeat.rs` | 0% | 95.25% | | `runner/inference.rs` | 0% | 76.38% | | `platform/unix.rs` | 38.24% | 94.48% | | `runner/config.rs` | 66.67% | 95.68% | | `runner/service_manager.rs` | 37.91% | 70.42% | | `commands/auth.rs` | 51.99% | 71.91% | ## Three defects, each reproduced before it was fixed **Runner token written world-readable.** `RunnerConfig::save` used a plain `fs::write` — 0644 under a default umask. The file holds `runner_token`, which can claim and report CI jobs, so on a shared runner host every account could read it. The user config in `src/config.rs` has always used create-at-0600-then-rename; the runner config now does too, which additionally tightens a file an older build left permissive and makes the save atomic. **Artifact paths could escape the workspace.** `resolve_paths` did `workspace.join(spec.path)`, and `Path::join` treats an absolute argument as a *replacement*, not an append — so a spec of `/etc/passwd` resolved to exactly that, and `../` climbed out just as easily. Artifact specs come from the built branch's `.anvil.yml`, so anyone able to open a pull request could have any runner-readable file uploaded to the server as a build artifact. Results are now confined to the workspace, canonicalizing both sides so a planted symlink can't smuggle a file out either. Worth a look on this one: it is a semantics change to a user-facing config surface. A pipeline that names an absolute artifact path now gets a loud `Refusing '<path>': ... is outside the job workspace` and no upload, where before it silently worked. **A blank line in SHA256SUMS disabled update verification.** `parse_checksum_line` used `?` on a line's first token, which returns from the whole function rather than skipping the line. One blank or comment line hid every entry after it; the update then failed with `no SHA256 entry`, and the documented way past that is `--no-verify`. A stray newline could talk a user out of verifying the downloaded binary at all. ## Quality work on already-green tests - `validate_instance_name` asserted only `is_err()`, so its three distinct rules could have collapsed into one unhelpful message undetected. Each is now pinned to the diagnostic it produces. - `verify_sha256` gained the rejection cases a supply-chain check needs: hash prefix, empty expected hash, and that the failure reports both digests. - Split a compound assert in `prepare.rs` so a failure names which half broke. ## Deliberately still at 0% `runner/shutdown.rs` installs process-wide signal handlers and `runner/detach.rs` daemonizes. Exercising either from the test process means raising real signals at, or forking, the test runner itself — the tests would be less trustworthy than the gap. ## Verification - `cargo test` — 364 lib + 251 integration, green - Stable across repeated runs and under `--test-threads=1` - `cargo clippy --all-targets --all-features -- -D warnings` — clean - `cargo fmt` applied - One test module went from 30.1s to 0.21s: it had been pointed at `127.0.0.1:1`, which WSL2 black-holes rather than refuses, so every probe burned a full timeout. Unreachable-endpoint tests now bind and drop an ephemeral port. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
SHA: 09ed84e155c2c96bfc94e0f2d289ee6afbea28eb
Author: Anvil <noreply@anvil.fangorn.io>
Date: 2026-08-05 16:19
Parents: 7ffcb3e
16 files changed +3664 -63
Type
docs/TESTING.md +249 −0
@@ -1,0 +1,249 @@
# A Manifesto on Excellence in Software Testing
*Written for anvil-cli, but the principles are not local to it.*
---
## 0. The premise
A test suite is not a safety net. A safety net is passive — it catches you after
you fall, and its only measure of quality is whether it has holes. A test suite is
an **executable specification with an adversary attached**. It states what the
program must do, and it actively tries to prove the program doesn't do it.
Everything below follows from taking that sentence literally.
---
## I. Coverage is a map of ignorance, not a measure of quality
Line coverage tells you exactly one thing: which lines were *executed*. It says
nothing about whether anything was *checked*. A test that runs a function and
asserts nothing raises coverage. A test that asserts `result.is_ok()` on a
function whose entire job is computing the value inside the `Ok` raises coverage
by exactly as much as a test that checks the value.
So:
- **Uncovered lines are a reliable signal.** They are proof of absence. Chase them.
- **Covered lines are not a reliable signal.** They are absence of proof. Audit them.
- **Never write a test to move the number.** The number is a byproduct of testing
well; the moment it becomes the goal, it stops measuring anything. A suite at
85% where every assertion is load-bearing is worth more than a suite at 98%
padded with `assert!(foo().is_ok())`.
The right use of a coverage report is as a **worklist**, read in this order:
1. Code that is uncovered *and* would be catastrophic if wrong (auth, secrets,
path handling, money, data destruction).
2. Code that is uncovered *and* pure (cheap to test, no excuse).
3. Code that is covered but only *incidentally* — executed as a side effect of
testing something else, with no assertion pointed at it. This is the most
dangerous category because the report says it's green.
---
## II. Assert on the consequence, not the mechanism
The question a test must answer is *"what would a user notice if this broke?"* —
and then assert on precisely that.
```rust
// Weak: passes if the function returns Ok having done nothing at all.
assert!(config.save(Some(path)).is_ok());
// Strong: passes only if the observable consequence actually happened.
config.save(Some(path)).unwrap();
let round_tripped = RunnerConfig::load(Some(path)).unwrap();
assert_eq!(round_tripped.runner_token, "secret");
assert_eq!(mode_bits(path), 0o600, "runner token must not be world-readable");
```
Corollaries:
- **Prefer round-trips to spot-checks.** `save` → `load` → compare proves both
halves and the format between them. Testing `save` by reading the file with a
hand-rolled parser tests your parser.
- **Assert the negative too.** "The secret is in the output" is half a test.
"…and the raw token is *not*" is the other half.
- **Test the error path's content, not just its existence.** An error that says
`"error"` and an error that says `"config not found at /x — run
\`anvil runner configure\` first"` are both `Err`. Only one of them is the
feature. If a diagnostic message is the product, assert on the message.
---
## III. A test that cannot fail is worse than no test
No test is honest ignorance. A test that cannot fail is a **false claim of
safety** — it occupies the slot where a real test would go, and it makes the
report say "checked."
Before a test is finished, it must pass this gate:
> **Break the code on purpose. Does this test go red?**
If you can't articulate a specific one-line mutation that turns the test red,
you haven't written a test — you've written an exercise. Common non-tests:
- `assert!(x.is_ok())` where the payload is the point.
- Asserting a `Vec` is non-empty when the bug would be wrong *contents*.
- `assert!(output.contains("error"))` — matches the word "error" anywhere,
including in an unrelated field or a success message about an error count.
- Comparing a value to itself through the code under test
(`assert_eq!(f(x), f(x))`).
- Snapshot tests blessed without ever being read.
---
## IV. Test at the seam where the meaning lives
Not everything deserves the same instrument.
| What it is | Where to test it | Why |
|---|---|---|
| Pure logic (parsers, path resolution, formatting, ID validation) | Unit test, in-module `#[cfg(test)]` | Fast, exhaustive, no fixtures. There is no excuse for an untested pure function. |
| Filesystem effects (perms, atomicity, rotation, PID files) | Unit test against a `TempDir` | The effect *is* the behavior; assert on the real filesystem, not a mock of it. |
| HTTP request shape (path, method, headers, body) | Mock server (`wiremock`) | Protocol contracts break silently and are caught by nothing else. |
| End-to-end output contracts (`--json`, exit codes) | Spawn the real binary | The user runs the binary, not a function. Only this catches "a stray `println!` corrupted the JSON stream." |
| Concurrency, signals, teardown | Deterministic harness with explicit synchronization | Never `sleep` and hope. |
The rule of thumb: **test as low as you can while still testing something a user
would care about.** Push a test lower and it gets faster and more precise; push
it too low and it starts asserting on implementation details that a legitimate
refactor will break. The seam you want is the lowest one where the assertion is
still phrased in the vocabulary of the requirement.
---
## V. The three questions that generate the good tests
For every function worth testing, ask:
1. **What does it do?** — the happy path. One test, and it's the least
interesting one you'll write.
2. **What are its edges?** — empty, zero, one, max, missing, duplicate, unicode,
the boundary and both sides of it. Most bugs live here.
3. **What does it do when it's attacked?** — the input the author didn't imagine.
`..` in a path. An absolute path where a relative one was assumed. A field
present but null. A field that is a string where a number was expected. A
response that is 200 with a body that says failure.
Question 3 is where the real defects are, and it's the one people skip because
it requires imagining a hostile world instead of a cooperative one. **The
happy-path test documents the feature; the adversarial test is the one that
finds the bug.**
A useful trigger: whenever you see `unwrap_or_default()`, `unwrap_or_else`, a
silent `continue`, a `filter_map` that drops errors, or `Path::join` on
externally-supplied data — stop. Something is being swallowed there, and the
test that proves what it swallows has almost certainly not been written.
---
## VI. Tests are read more than they are run
The failure output of a test is a bug report written in advance, addressed to
someone at 3am who did not write the code. Optimize for that reader.
- **The name is the specification.** `test_config()` says nothing.
`save_writes_runner_token_with_owner_only_permissions()` tells the reader what
broke without opening the file.
- **Put the diagnosis in the assertion message.** Include the actual value, the
expected value, and enough context to act. A test asserting on subprocess
output should dump stdout *and* stderr on failure — otherwise the first thing
the reader must do is edit the test.
- **One behavior per test.** A test with six assertions reports only the first
failure; the other five stay hidden until you fix and rerun, five times.
- **No logic in tests.** A loop or conditional in a test is a second program that
can itself be wrong, and it is not tested. Table-driven cases are fine — the
*table* is data; a branch inside the assertion is not.
- **Comment the *why*, never the *what*.** `// regression: #375 — a 200 with an
empty body used to be treated as success` is worth more than the test body.
---
## VII. Determinism is non-negotiable
A flaky test is worse than a deleted test, because it teaches the team to ignore
red. The response to a flake is to fix the determinism or delete the test — never
to rerun until green.
The usual sources, and their fixes:
- **Time** → inject a clock; never assert on wall-clock durations.
- **Sleeps used as synchronization** → wait on the actual condition with a bounded
poll and a timeout, or use a real notification primitive.
- **Shared global state** — the real filesystem, `$HOME`, env vars, fixed ports,
a shared DB → give each test its own `TempDir`, its own ephemeral port, its own
scoped state. Rust runs tests in parallel *by default*; a test that mutates
process-global state is a test that will randomly fail its neighbors.
- **Ordering assumptions** on hash-map iteration, filesystem `read_dir`, or
concurrent completion → sort before asserting, or assert on sets.
- **The network** → mock it. A test that reaches the real internet is a test that
fails on a plane.
Every test must pass alone, in a full run, in a random order, and a thousand
times in a row.
---
## VIII. A bug that escaped is a missing test, and the test comes first
When a defect is found in the wild, the sequence is fixed:
1. Write the test that reproduces it. **Watch it fail.** This is the only moment
you can be certain the test is actually pointed at the bug.
2. Fix the code.
3. Watch it pass.
4. Leave the issue number in a comment on the test.
The value of step 1 is not ceremony. A test written after the fix is a test that
has never been observed to fail, which by §III is not yet known to be a test.
---
## IX. Do not test the language, the framework, or the mock
Effort spent asserting that `serde` deserializes, that `clap` parses `--flag`, or
that the mock returns what you configured it to return is effort not spent on
your logic. Test *your* code's contribution: the defaults you supplied, the
validation you added, the shape you chose.
The special case of this that matters most: **do not let a mock become the
subject.** If a test only proves "the mock was called," it will keep passing
forever after the real server changes its contract. Assert on request *shape* —
the path, the method, the body the server will actually receive — because that's
the part that has to stay true of the real system.
---
## X. When you touch code, you owe it a test
Not a rule about ceremony — a rule about ratchets. A codebase's test quality only
ever moves in the direction of the marginal change. If every change leaves the
tested surface slightly better than it found it, quality compounds. If changes
are allowed to leave it flat, it decays, because the code around it keeps
growing.
The concrete obligation, in priority order:
1. If you fixed a bug, §VIII applies. No exceptions.
2. If you added a branch, test both sides of it.
3. If you touched a function with a weak existing test, strengthen the assertion
while you're there. This is the cheapest quality improvement that exists —
the context is already loaded in your head.
4. If you found a function that is pure, public, and untested, and it took you
thirty seconds to see what it does, it takes ninety seconds to test it. Do it.
---
## The short version
> Uncovered code is a worklist; covered code is an unaudited claim.
> Assert on consequences, not mechanisms.
> If you can't name the mutation that turns it red, it isn't a test.
> The happy path documents; the hostile input finds.
> Write the failure message for a stranger at 3am.
> Flaky is worse than absent.
> Reproduce before you fix.
src/commands/auth.rs +149 −0
@@ -607,4 +607,153 @@
other => panic!("expected Other, got {other:?}"),
}
}
// ── verify_token ──────────────────────────────────────────────────
//
// This is the gate between "the user pasted a token" and "the token is on
// disk": `Ok(_)` means save it, `Err` means refuse. Getting the split
// wrong either strands a working login or persists a dead credential.
mod verify {
use super::super::verify_token;
use serde_json::json;
use wiremock::matchers::{method, path as req_path};
use wiremock::{Mock, MockServer, ResponseTemplate};
/// A server whose `/api/v1/users/me` answers with `status` and `body`.
async fn users_me(status: u16, body: serde_json::Value) -> MockServer {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/v1/users/me"))
.respond_with(ResponseTemplate::new(status).set_body_json(body))
.mount(&server)
.await;
server
}
#[tokio::test]
async fn a_valid_token_yields_the_username_for_the_confirmation_line() {
let server = users_me(200, json!({"username": "cole"})).await;
let who = verify_token(&server.uri(), "anvil_good").await.unwrap();
assert_eq!(who.as_deref(), Some("cole"));
}
#[tokio::test]
async fn the_username_is_found_however_the_server_nests_it() {
// Three response shapes have shipped; all must still resolve.
for body in [
json!({"username": "cole"}),
json!({"user": {"username": "cole"}}),
json!({"data": {"username": "cole"}}),
] {
let server = users_me(200, body.clone()).await;
let who = verify_token(&server.uri(), "anvil_good").await.unwrap();
assert_eq!(who.as_deref(), Some("cole"), "shape was: {body}");
}
}
#[tokio::test]
async fn the_token_is_sent_as_a_bearer_credential() {
let server = users_me(200, json!({"username": "cole"})).await;
verify_token(&server.uri(), "anvil_good").await.unwrap();
let reqs = server.received_requests().await.unwrap();
assert_eq!(
reqs[0].headers.get("authorization").unwrap(),
"Bearer anvil_good"
);
}
#[tokio::test]
async fn a_success_without_a_username_still_saves_the_token() {
// The identity is cosmetic; a server that omits it must not cost
// the user a working login.
let server = users_me(200, json!({"id": 7})).await;
let who = verify_token(&server.uri(), "anvil_good").await.unwrap();
assert!(who.is_none(), "got: {who:?}");
}
#[tokio::test]
async fn a_rejected_token_is_refused_and_the_error_says_it_was_not_saved() {
// Persisting a token the server just refused leaves the CLI broken
// in a way that looks like a server outage.
for status in [401u16, 403] {
let server = users_me(status, json!({"error": "unauthorized"})).await;
let err = verify_token(&server.uri(), "anvil_bad")
.await
.err()
.unwrap_or_else(|| panic!("HTTP {status} must refuse the token"))
.to_string();
assert!(
err.contains("Not saved"),
"HTTP {status} must tell the user nothing was written, got: {err}"
);
assert!(
err.contains(&server.uri()),
"the error must name the server it asked, got: {err}"
);
}
}
#[tokio::test]
async fn a_server_error_is_not_treated_as_a_rejection() {
// A 500 says nothing about the token. Refusing here would make a
// transient outage look like a bad credential.
let server = users_me(500, json!({"error": "boom"})).await;
let who = verify_token(&server.uri(), "anvil_good")
.await
.expect("a 5xx must not reject the token");
assert!(who.is_none());
}
#[tokio::test]
async fn an_unreachable_server_does_not_block_saving_the_token() {
// `anvil auth login --token` has to work offline, or setting up a
// machine before the VPN is up becomes impossible.
let listener =
std::net::TcpListener::bind("127.0.0.1:0").expect("bind an ephemeral port");
let port = listener.local_addr().unwrap().port();
drop(listener);
let who = verify_token(&format!("http://127.0.0.1:{port}"), "anvil_good")
.await
.expect("an unreachable server must not reject the token");
assert!(who.is_none());
}
#[tokio::test]
async fn a_non_json_success_body_does_not_reject_the_token() {
// A proxy login page in front of the API returns 200 and HTML.
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/v1/users/me"))
.respond_with(ResponseTemplate::new(200).set_body_string("<html>hi</html>"))
.mount(&server)
.await;
let who = verify_token(&server.uri(), "anvil_good").await.unwrap();
assert!(who.is_none());
}
#[tokio::test]
async fn a_non_string_username_is_ignored_rather_than_rendered() {
let server = users_me(200, json!({"username": 42})).await;
let who = verify_token(&server.uri(), "anvil_good").await.unwrap();
assert!(who.is_none(), "got: {who:?}");
}
}
}
src/commands/update.rs +190 −13
@@ -152,18 +152,37 @@
/// Parse a single SHA256SUMS entry. Lines look like `<hash> <filename>` —
/// standard `sha256sum` output. Returns the hash for the matching filename.
///
/// A line that isn't in that shape (blank, or a lone comment/header token) is
/// skipped rather than ending the scan: abandoning the whole document on the
/// first odd line hides every entry after it, and the resulting "no SHA256
/// entry" failure pushes the user towards `--no-verify`.
///
/// The first field must actually look like a SHA-256. Skipping malformed lines
/// means the scan now walks a document that may not be a checksums file at all
/// — a proxy error page, release notes, a directory index — where some prose
/// line can easily have the filename as its second whitespace token. Without
/// this check a bullet like `* anvil_linux_amd64_1.4.0 (2026-08-01)` yields the
/// "expected hash" `*`, and the update fails with a mismatch that reads like a
/// tampered download instead of the accurate "no SHA256 entry".
fn parse_checksum_line(body: &str, filename: &str) -> Option<String> {
for line in body.lines() {
let mut parts = line.split_whitespace();
let hash = parts.next()?;
let name = parts.next()?;
if name == filename {
let (Some(hash), Some(name)) = (parts.next(), parts.next()) else {
continue;
};
if name == filename && is_sha256_hex(hash) {
return Some(hash.to_string());
}
}
None
}
/// Exactly the 64 hex characters a SHA-256 digest is written as.
fn is_sha256_hex(s: &str) -> bool {
s.len() == 64 && s.bytes().all(|b| b.is_ascii_hexdigit())
}
fn verify_sha256(bytes: &[u8], expected_hex: &str) -> Result<(), Box<dyn std::error::Error>> {
let mut hasher = Sha256::new();
hasher.update(bytes);
@@ -258,28 +277,39 @@
mod tests {
use super::*;
/// A digest-shaped 64-hex value. The old fixtures used stand-ins like
/// "deadbeef"; now that the parser insists on a real SHA-256 shape, a
/// fixture that isn't one would test the rejection path by accident.
fn digest(seed: char) -> String {
std::iter::repeat_n(seed, 64).collect()
}
#[test]
fn parse_checksum_line_matches_filename() {
let (a, b, c) = (digest('a'), digest('b'), digest('c'));
let body = "abc123 anvil_linux_amd64_2026.05.6\n\
let body = format!(
"{a} anvil_linux_amd64_2026.05.6\n\
{b} anvil_linux_arm64_2026.05.6\n\
{c} SHA256SUMS_2026.05.6\n"
);
deadbeef anvil_linux_arm64_2026.05.6\n\
cafebabe SHA256SUMS_2026.05.6\n";
assert_eq!(
parse_checksum_line(body, "anvil_linux_arm64_2026.05.6").as_deref(),
Some("deadbeef")
parse_checksum_line(&body, "anvil_linux_arm64_2026.05.6").as_deref(),
Some(b.as_str())
);
}
#[test]
fn parse_checksum_line_missing_returns_none() {
let body = "abc anvil_linux_amd64_1.0.0\n";
assert!(parse_checksum_line(body, "anvil_linux_arm64_1.0.0").is_none());
let body = format!("{} anvil_linux_amd64_1.0.0\n", digest('a'));
assert!(parse_checksum_line(&body, "anvil_linux_arm64_1.0.0").is_none());
}
#[test]
fn parse_checksum_tolerates_extra_whitespace() {
let a = digest('a');
let body = " abc123 anvil_linux_amd64_1.0.0 \n";
let body = format!(" {a} anvil_linux_amd64_1.0.0 \n");
assert_eq!(
parse_checksum_line(body, "anvil_linux_amd64_1.0.0").as_deref(),
parse_checksum_line(&body, "anvil_linux_amd64_1.0.0").as_deref(),
Some(a.as_str())
Some("abc123")
);
}
@@ -310,5 +340,152 @@
fn verify_sha256_rejects_wrong_hash() {
let err = verify_sha256(b"hello", "deadbeef").unwrap_err().to_string();
assert!(err.contains("SHA256 mismatch"));
}
// ── supply chain ──────────────────────────────────────────────────
//
// Verification is the only thing between a tampered download and
// `swap_executable` overwriting the running binary, so the rejection
// cases deserve pinning individually.
#[test]
fn verify_sha256_reports_both_hashes_so_a_mismatch_can_be_investigated() {
// Without the actual digest an operator cannot tell a corrupted
// download from a stale checksums file.
let err = verify_sha256(b"hello world", "deadbeef")
.unwrap_err()
.to_string();
assert!(
err.contains("deadbeef"),
"expected hash missing from: {err}"
);
assert!(
err.contains("b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"),
"actual hash missing from: {err}"
);
}
#[test]
fn verify_sha256_rejects_a_prefix_of_the_correct_hash() {
// A truncated entry must never be accepted as a match.
assert!(verify_sha256(b"hello world", "b94d27b9").is_err());
}
#[test]
fn verify_sha256_rejects_an_empty_expected_hash() {
// Belt and braces: an empty string must not verify, whatever a
// malformed checksums file hands us.
assert!(verify_sha256(b"hello world", "").is_err());
assert!(verify_sha256(b"", "").is_err());
}
#[test]
fn parse_checksum_line_keeps_looking_past_a_blank_line() {
// Regression: the scan used `?` on a line's first token, so one blank
// line returned None for the whole document instead of skipping.
// Every entry after it became invisible, the update aborted with "no
// SHA256 entry", and the documented way past that is `--no-verify` —
// i.e. a stray newline talks the user out of verifying at all.
let (a, b) = (digest('a'), digest('b'));
let body = format!(
"{a} anvil_linux_amd64_1.0.0\n\
\n\
{b} anvil_linux_arm64_1.0.0\n"
);
assert_eq!(
parse_checksum_line(&body, "anvil_linux_arm64_1.0.0").as_deref(),
Some(b.as_str()),
"a blank line must not hide the entries after it"
);
}
#[test]
fn parse_checksum_line_keeps_looking_past_a_single_token_line() {
// The same shape, arriving as a comment or a bare header line.
let b = digest('b');
let body = format!("#anvil-release-checksums\n{b} anvil_linux_arm64_1.0.0\n");
assert_eq!(
parse_checksum_line(&body, "anvil_linux_arm64_1.0.0").as_deref(),
Some(b.as_str())
);
}
#[test]
fn a_prose_body_yields_no_hash_rather_than_a_garbage_one() {
// Skipping malformed lines means the scan walks documents that are not
// checksums files at all. A proxy error page or a release-notes body
// can easily carry the filename as a line's second token; taking the
// first token as "the expected hash" turns a benign "no SHA256 entry"
// into `SHA256 mismatch: expected *`, which reads as a tampered
// download and argues for --no-verify.
let body = "<h1>Releases</h1>\n\
* anvil_linux_amd64_1.4.0 (2026-08-01)\n\
- anvil_linux_amd64_1.4.0 signed by release-bot\n";
assert!(
parse_checksum_line(body, "anvil_linux_amd64_1.4.0").is_none(),
"a non-digest token must never be returned as an expected hash"
);
}
#[test]
fn a_truncated_or_non_hex_digest_is_not_accepted_as_an_entry() {
for bad in [
"deadbeef",
&"a".repeat(63),
&"a".repeat(65),
&"z".repeat(64),
] {
let body = format!("{bad} anvil_linux_amd64_1.0.0\n");
assert!(
parse_checksum_line(&body, "anvil_linux_amd64_1.0.0").is_none(),
"{bad} is not a SHA-256 and must not be returned as one"
);
}
}
#[test]
fn parse_checksum_line_requires_an_exact_filename_match() {
// A prefix match would let `anvil_linux_amd64_1.0.0` answer the
// lookup for `anvil_linux_amd64_1.0.0-rc1` and verify the wrong file.
let body = format!("{} anvil_linux_amd64_1.0.0\n", digest('a'));
assert!(parse_checksum_line(&body, "anvil_linux_amd64_1.0.0-rc1").is_none());
assert!(parse_checksum_line(&body, "anvil_linux_amd64_1.0").is_none());
}
#[test]
fn parse_checksum_line_returns_the_first_entry_for_a_duplicated_filename() {
let (a, b) = (digest('a'), digest('b'));
let body = format!(
"{a} anvil_linux_amd64_1.0.0\n\
{b} anvil_linux_amd64_1.0.0\n"
);
assert_eq!(
parse_checksum_line(&body, "anvil_linux_amd64_1.0.0").as_deref(),
Some(a.as_str())
);
}
#[test]
fn parse_checksum_line_of_an_empty_document_is_none() {
assert!(parse_checksum_line("", "anvil_linux_amd64_1.0.0").is_none());
}
#[test]
fn parse_checksum_line_does_not_match_a_binary_marked_entry() {
// `sha256sum -b` writes `<hash> *<name>`; the star belongs to the
// second field, so such an entry must not be taken for a plain one.
let body = format!("{} *anvil_linux_amd64_1.0.0\n", digest('a'));
assert!(
parse_checksum_line(&body, "anvil_linux_amd64_1.0.0").is_none(),
"a binary-marked entry must not be mistaken for a plain one"
);
}
}
src/lib.rs +3 −0
@@ -12,3 +12,6 @@
pub mod platform;
pub mod runner;
#[cfg(test)]
pub mod testutil;
src/platform/unix.rs +283 −1
@@ -14,9 +14,28 @@
return false;
}
let rc = unsafe { libc::kill(pid, 0) };
let errno = if rc == 0 {
0
} else {
std::io::Error::last_os_error().raw_os_error().unwrap_or(0)
};
alive_from_kill(rc, errno)
}
/// How to read the result of `kill(pid, 0)`.
///
/// Split out because the EPERM arm cannot be reached on demand from a test:
/// it needs a live process this account may not signal, and under root — which
/// is how the suite runs in CI's container — there is no such process, so
/// `kill` succeeds outright and the arm is never evaluated. Deleting it
/// entirely would leave the suite green. As a pure mapping it can be checked
/// directly, whatever uid the tests happen to run as.
fn alive_from_kill(rc: i32, errno: i32) -> bool {
if rc == 0 {
return true;
}
let errno = std::io::Error::last_os_error().raw_os_error().unwrap_or(0);
// EPERM means the process exists but belongs to somebody else. Reading it
// as "gone" would have a non-root `runner status` declare a live daemon
// crashed and start a second one on the same work dir.
errno == libc::EPERM
}
@@ -59,4 +78,267 @@
/// Program + leading args used to run an arbitrary shell command string.
pub fn bare_shell() -> (&'static str, Vec<&'static str>) {
("/bin/sh", vec!["-c"])
}
#[cfg(test)]
mod tests {
use super::*;
use std::process::{Command, Stdio};
/// A child that will not exit on its own, so a liveness assertion is
/// about our signalling rather than a race with its own exit.
///
/// Wrapped in a guard that kills and reaps on drop: a test that panics
/// before it gets to its own `kill` would otherwise leave a `sleep 30`
/// behind on every failing run, and these runners are long-lived.
struct Sleeper(Option<std::process::Child>);
impl Sleeper {
fn new() -> Self {
Self(Some(
Command::new("/bin/sh")
.args(["-c", "sleep 30"])
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
.expect("spawn a sleeper"),
))
}
fn pid(&self) -> u32 {
self.0.as_ref().expect("child still owned").id()
}
/// The signal this child was killed by, reaping it in the process.
///
/// `wait` blocks until it actually exits, which is the deterministic
/// way to observe a signal — polling `is_alive` after a `kill` is both
/// racy and wrong, since a killed-but-unreaped child is a zombie and
/// `kill(pid, 0)` still succeeds on a zombie.
fn killed_by(&mut self) -> Option<i32> {
use std::os::unix::process::ExitStatusExt;
self.0
.as_mut()
.expect("child still owned")
.wait()
.expect("reap the child")
.signal()
}
}
impl Drop for Sleeper {
fn drop(&mut self) {
if let Some(mut child) = self.0.take() {
let _ = child.kill();
let _ = child.wait();
}
}
}
/// A pid that has been spawned, exited and reaped — so it is definitely
/// not alive, without inventing a number that might belong to somebody.
fn reaped_pid() -> i32 {
let mut child = Command::new("/bin/sh")
.args(["-c", "exit 0"])
.spawn()
.unwrap();
let pid = child.id() as i32;
child.wait().unwrap();
pid
}
#[test]
fn a_nonsense_pid_is_never_alive() {
// `kill(0, …)` signals our whole process group and `kill(-1, …)`
// every process we may signal. Both would report a bogus "alive",
// and both are catastrophic to pass on to a real signal.
assert!(!is_alive(0), "pid 0 means the process group, not a process");
assert!(!is_alive(-1), "pid -1 means every process");
assert!(!is_alive(-12345));
}
#[test]
fn this_very_process_is_alive() {
assert!(is_alive(std::process::id() as i32));
}
#[test]
fn a_process_that_exists_but_is_not_ours_to_signal_counts_as_alive() {
// The EPERM arm, checked directly. Going through `is_alive(1)` instead
// proves nothing wherever the suite runs as root — CI's container does
// — because there `kill(1, 0)` simply succeeds and this arm is never
// evaluated, so deleting it would leave the suite green.
assert!(
alive_from_kill(-1, libc::EPERM),
"EPERM means the process exists and belongs to another account"
);
}
#[test]
fn a_process_that_does_not_exist_is_not_alive() {
assert!(!alive_from_kill(-1, libc::ESRCH));
}
#[test]
fn a_successful_probe_means_alive() {
assert!(alive_from_kill(0, 0));
}
#[test]
fn an_unexpected_errno_is_not_read_as_alive() {
// Only EPERM is evidence of existence; anything else is not.
assert!(!alive_from_kill(-1, libc::EINVAL));
assert!(!alive_from_kill(-1, 0));
}
#[test]
fn pid_one_is_alive_whichever_arm_answers() {
// End-to-end sanity over the real syscall: as root this passes via
// rc == 0, unprivileged via EPERM. Both are correct; the arms
// themselves are pinned above.
assert!(is_alive(1));
}
#[test]
fn a_reaped_child_is_not_alive() {
let pid = reaped_pid();
assert!(
!is_alive(pid),
"reaped pid {pid} still reads as alive; stale PID files would never be cleaned up"
);
}
#[test]
fn a_killed_but_unreaped_child_still_reads_as_alive() {
// `kill(pid, 0)` succeeds on a zombie, so `is_alive` answers
// "does this pid exist", not "is this process running". Anything
// deciding whether a runner is up must reap or check elsewhere —
// this is a property to know about, not a bug to route around.
let mut child = Sleeper::new();
let pid = child.pid() as i32;
force_kill(pid).unwrap();
std::thread::sleep(std::time::Duration::from_millis(200));
assert!(
is_alive(pid),
"expected the zombie at {pid} to still satisfy kill(pid, 0)"
);
assert_eq!(child.killed_by(), Some(libc::SIGKILL));
assert!(!is_alive(pid), "reaping must retire the pid");
}
#[test]
fn force_kill_terminates_a_running_process() {
let mut child = Sleeper::new();
let pid = child.pid() as i32;
assert!(is_alive(pid), "the sleeper should have started");
force_kill(pid).expect("SIGKILL to our own child must succeed");
assert_eq!(
child.killed_by(),
Some(libc::SIGKILL),
"the sleeper did not die of SIGKILL"
);
}
#[test]
fn force_kill_of_an_absent_process_is_an_error_not_a_silent_success() {
// `runner stop` surfaces this; swallowing it would report having
// killed a runner that was never running.
let pid = reaped_pid();
assert!(
force_kill(pid).is_err(),
"expected an error for reaped pid {pid}"
);
}
#[test]
fn request_graceful_stop_sends_a_signal_the_process_actually_dies_from() {
// SIGTERM's default action is terminate, so a child that installs no
// handler must go away — otherwise `runner stop` hangs forever.
let mut child = Sleeper::new();
let pid = child.pid() as i32;
request_graceful_stop(pid, std::path::Path::new("/unused/on/unix.pid"))
.expect("SIGTERM to our own child must succeed");
assert_eq!(
child.killed_by(),
Some(libc::SIGTERM),
"runner stop must send SIGTERM, not something the child ignores"
);
}
#[test]
fn request_graceful_stop_reports_a_process_that_is_already_gone() {
let pid = reaped_pid();
assert!(request_graceful_stop(pid, std::path::Path::new("/x")).is_err());
}
#[test]
fn request_child_terminate_stops_a_child_we_spawned() {
let mut child = Sleeper::new();
request_child_terminate(child.pid());
assert_eq!(
child.killed_by(),
Some(libc::SIGTERM),
"the executor's timeout path never terminated the child"
);
}
#[test]
fn request_child_terminate_of_an_absent_process_does_not_panic() {
// Best-effort by contract — the caller escalates to SIGKILL.
request_child_terminate(reaped_pid() as u32);
}
#[test]
fn is_elevated_agrees_with_the_id_command() {
// Comparing against `libc::getuid` would only restate the
// implementation; `id -u` is an independent oracle.
let out = Command::new("id").arg("-u").output().expect("run id -u");
let uid: u32 = String::from_utf8_lossy(&out.stdout).trim().parse().unwrap();
assert_eq!(
is_elevated(),
uid == 0,
"is_elevated disagrees with `id -u` ({uid})"
);
}
#[test]
fn bare_shell_names_a_shell_and_the_flag_that_takes_a_command_string() {
let (program, args) = bare_shell();
assert_eq!(program, "/bin/sh");
assert_eq!(args, vec!["-c"]);
}
#[test]
fn bare_shell_actually_runs_a_command_string() {
// The tuple is only correct if it composes into a working
// invocation; asserting the strings alone would not catch a flag
// that stopped meaning "read the next argument as a command".
let (program, args) = bare_shell();
let out = Command::new(program)
.args(&args)
.arg("echo composed && exit 3")
.output()
.expect("run through the bare shell");
assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "composed");
assert_eq!(
out.status.code(),
Some(3),
"the shell's exit status must reach the caller"
);
}
}
src/runner/artifacts.rs +861 −33
@@ -29,44 +29,75 @@
.collect()
}
/// What an artifact-upload pass did, and anything the job's author needs to
/// be told about it.
///
/// `notices` exists because the interesting outcomes here are refusals, and a
/// refusal explained only on the runner host's stderr is invisible to the
/// person whose pipeline it affected. The caller writes these into the job log.
pub struct UploadOutcome {
pub uploaded: u32,
pub notices: Vec<String>,
}
/// Upload all artifacts matching the specs from the workspace directory.
///
/// Resolves paths relative to workspace, supports glob patterns.
/// Resolves paths relative to workspace, supports glob patterns, and confines
/// every result to the workspace (see [`confine_to_workspace`]).
/// Skips files over MAX_ARTIFACT_SIZE with a warning.
/// Returns the number of successfully uploaded artifacts.
pub async fn upload_artifacts(
config: &RunnerConfig,
job_id: &str,
workspace: &Path,
specs: &[ArtifactSpec],
) -> u32 {
) -> UploadOutcome {
let mut notices = Vec::new();
if specs.is_empty() {
return 0;
return UploadOutcome {
uploaded: 0,
notices,
};
}
// Resolve the workspace once: it is both the confinement boundary and the
// base that multi-file artifact names are relative to, and those two must
// agree or a confined path won't strip.
let Ok(root) = workspace.canonicalize() else {
notices.push(format!(
"Collected no artifacts: workspace {} does not resolve",
workspace.display()
));
return UploadOutcome {
uploaded: 0,
notices,
};
};
let client = reqwest::Client::new();
let mut uploaded = 0u32;
for spec in specs {
let resolved = resolve_paths(&root, &spec.path);
notices.extend(resolved.refusals);
let files = resolve_paths(workspace, &spec.path);
let files = resolved.files;
if files.is_empty() {
eprintln!(
" [artifacts] Warning: no files matched '{}' for artifact '{}'",
notices.push(format!(
"No files matched '{}' for artifact '{}'",
spec.path, spec.name
));
);
continue;
}
// Name from the shape of the *spec*, not from how many files happened
// to survive: a glob is always namespaced, so dropping one match can
// never silently rename the artifact its sibling is published under.
let namespaced = is_glob(&spec.path);
for file_path in &files {
let artifact_name = if files.len() > 1 {
// Multiple files: use relative path as name
let rel = file_path
.strip_prefix(workspace)
.unwrap_or(file_path)
.to_string_lossy()
.to_string();
format!("{}/{}", spec.name, rel)
let artifact_name = if namespaced {
let rel = file_path.strip_prefix(&root).unwrap_or(file_path);
format!("{}/{}", spec.name, rel.to_string_lossy())
} else {
spec.name.clone()
};
@@ -77,45 +108,156 @@
uploaded += 1;
}
Err(e) => {
eprintln!(" [artifacts] Failed to upload '{}': {}", artifact_name, e);
notices.push(format!("Failed to upload '{artifact_name}': {e}"));
}
}
}
}
for n in &notices {
eprintln!(" [artifacts] {n}");
uploaded
}
UploadOutcome { uploaded, notices }
}
/// Resolve a path pattern relative to workspace.
/// Supports simple glob patterns (*, **).
/// Whether a spec path is a pattern rather than a single literal file.
fn resolve_paths(workspace: &Path, pattern: &str) -> Vec<PathBuf> {
let full_pattern = workspace.join(pattern);
let pattern_str = full_pattern.to_string_lossy();
fn is_glob(pattern: &str) -> bool {
pattern.contains('*') || pattern.contains('?') || pattern.contains('[')
}
/// Files a spec resolved to, plus the reasons anything was turned away.
// Check if it's a glob pattern
if pattern_str.contains('*') || pattern_str.contains('?') || pattern_str.contains('[') {
match glob::glob(&pattern_str) {
struct Resolved {
files: Vec<PathBuf>,
refusals: Vec<String>,
}
/// Resolve a path pattern relative to `root` (already canonicalized).
///
/// Every returned path is itself canonical and confined to the workspace —
/// see [`confine_to_workspace`].
fn resolve_paths(root: &Path, pattern: &str) -> Resolved {
let mut refusals = Vec::new();
let candidates = if is_glob(pattern) {
let full_pattern = root.join(pattern);
match glob::glob(&full_pattern.to_string_lossy()) {
Ok(paths) => paths
.filter_map(Result::ok)
.filter(|p| p.is_file())
.collect(),
Err(e) => {
eprintln!(" [artifacts] Invalid glob pattern '{}': {}", pattern, e);
refusals.push(format!("Invalid glob pattern '{pattern}': {e}"));
Vec::new()
}
}
} else {
let path = root.join(pattern);
// Literal path
let path = workspace.join(pattern);
if path.is_file() {
vec![path]
} else {
Vec::new()
}
};
let files = confine_to_workspace(root, candidates, pattern, &mut refusals);
Resolved { files, refusals }
}
/// Keep only files that really live inside the workspace, and return them in
/// their *resolved* form.
///
/// `Path::join` treats an absolute argument as a replacement, not an append:
/// `workspace.join("/etc/passwd")` is simply `/etc/passwd`, and a `../` prefix
/// escapes just as easily. Artifact specs originate in the built branch's
/// `.anvil.yml`, so without this an ordinary contributor can name any file the
/// runner account can read and have it uploaded as a build artifact.
///
/// Three distinct escapes are closed:
///
/// * **Symlinks** — both sides are canonicalized before comparing, so a link
/// planted in the workspace cannot point out of it.
/// * **Re-resolution** — the *canonical* path is what gets returned and
/// later opened. Returning the unresolved path would mean the file that is
/// read is not the file that was checked, and the workspace stays writable
/// (a container that outlived its timeout still has it bind-mounted) right
/// up until the upload.
/// * **Hardlinks** — a hardlink has no link to resolve, so it canonicalizes
/// inside the workspace and would otherwise sail through. A link count
/// above one, or a device that differs from the workspace's, means the
/// bytes may live outside the tree we are willing to publish.
///
/// `Path::starts_with` compares whole components, so a sibling directory named
/// like the workspace plus a suffix (`/work-evil` against `/work`) is not a
/// prefix match.
fn confine_to_workspace(
root: &Path,
files: Vec<PathBuf>,
pattern: &str,
refusals: &mut Vec<String>,
) -> Vec<PathBuf> {
#[cfg(unix)]
let root_dev = std::fs::metadata(root)
.map(|m| {
use std::os::unix::fs::MetadataExt;
m.dev()
})
.ok();
let mut kept = Vec::new();
for p in files {
let Ok(real) = p.canonicalize() else {
continue;
};
if !real.starts_with(root) {
refusals.push(format!(
"Refusing '{pattern}': {} is outside the job workspace",
real.display()
));
continue;
}
#[cfg(unix)]
{
use std::os::unix::fs::MetadataExt;
let Ok(meta) = std::fs::metadata(&real) else {
continue;
};
if meta.nlink() > 1 {
refusals.push(format!(
"Refusing '{pattern}': {} has {} hard links, so its contents \
may also exist outside the workspace — copy it instead of linking",
real.display(),
meta.nlink()
));
continue;
}
if root_dev.is_some_and(|d| d != meta.dev()) {
refusals.push(format!(
"Refusing '{pattern}': {} is on a different filesystem from the \
job workspace",
real.display()
));
continue;
}
}
kept.push(real);
}
kept
}
/// Upload a single file as a multipart form to the artifact endpoint.
///
/// The file is opened **once** and both the size check and the read are served
/// from that handle, so the bytes that are sent are the bytes that were
/// measured — a stat-then-read pair leaves a window in which the path can be
/// swapped. On Unix the open additionally refuses to follow a symlink at the
/// final component: the caller resolved this path already, so a link appearing
/// there now is something that changed underneath us.
async fn upload_single(
client: &reqwest::Client,
config: &RunnerConfig,
@@ -123,8 +265,21 @@
file_path: &Path,
artifact_name: &str,
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
// Check file size
let metadata = std::fs::metadata(file_path)?;
use std::io::Read;
let mut opts = std::fs::OpenOptions::new();
opts.read(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.custom_flags(libc::O_NOFOLLOW);
}
let mut file = opts.open(file_path)?;
let metadata = file.metadata()?;
if !metadata.is_file() {
return Err("not a regular file".into());
}
if metadata.len() > MAX_ARTIFACT_SIZE {
return Err(format!(
"file too large ({} bytes, max {} bytes)",
@@ -134,11 +289,11 @@
.into());
}
let file_bytes = std::fs::read(file_path)?;
let mut file_bytes = Vec::with_capacity(metadata.len() as usize);
file.read_to_end(&mut file_bytes)?;
let url = config.api_url(&format!("/runners/jobs/{job_id}/artifacts"));
// Try multipart upload first
let file_part = multipart::Part::bytes(file_bytes)
.file_name(artifact_name.to_string())
.mime_str("application/octet-stream")?;
@@ -160,5 +315,678 @@
} else {
let body = resp.text().await.unwrap_or_default();
Err(format!("upload failed ({status}): {body}").into())
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::testutil::TempDir;
use serde_json::json;
use wiremock::matchers::{method, path as req_path};
use wiremock::{Mock, MockServer, ResponseTemplate};
fn names(specs: &[ArtifactSpec]) -> Vec<(&str, &str)> {
specs
.iter()
.map(|s| (s.name.as_str(), s.path.as_str()))
.collect()
}
/// resolve_paths takes an already-canonical root, as upload_artifacts
/// resolves it once up front.
fn resolve(ws: &TempDir, pattern: &str) -> Resolved {
resolve_paths(&ws.canonical(), pattern)
}
fn file_names(r: &Resolved) -> Vec<String> {
let mut v: Vec<String> = r
.files
.iter()
.map(|p| p.file_name().unwrap().to_string_lossy().to_string())
.collect();
v.sort();
v
}
// ── parse_specs ───────────────────────────────────────────────────
#[test]
fn specs_are_read_in_order_from_the_claim_response() {
let job = json!({"artifact_specs": [
{"name": "coverage", "path": "cover/"},
{"name": "logs", "path": "tmp/*.log"}
]});
let specs = parse_specs(&job);
assert_eq!(
names(&specs),
vec![("coverage", "cover/"), ("logs", "tmp/*.log")]
);
}
#[test]
fn a_job_with_no_artifact_specs_collects_nothing() {
assert!(parse_specs(&json!({})).is_empty());
assert!(parse_specs(&json!({"artifact_specs": null})).is_empty());
}
#[test]
fn a_non_array_artifact_specs_field_is_ignored_rather_than_panicking() {
// A server sending the wrong shape must not take the runner down
// mid-job — the build already succeeded by this point.
assert!(parse_specs(&json!({"artifact_specs": "cover/"})).is_empty());
assert!(parse_specs(&json!({"artifact_specs": {"name": "x"}})).is_empty());
}
#[test]
fn a_spec_missing_name_or_path_is_skipped_but_its_siblings_survive() {
// Dropping the whole list would lose good artifacts to one bad entry.
let job = json!({"artifact_specs": [
{"path": "no-name.txt"},
{"name": "no-path"},
{"name": "good", "path": "out.txt"},
{"name": 7, "path": "wrong-type.txt"}
]});
assert_eq!(names(&parse_specs(&job)), vec![("good", "out.txt")]);
}
// ── is_glob ───────────────────────────────────────────────────────
#[test]
fn glob_detection_drives_artifact_naming_so_it_must_match_resolution() {
// resolve_paths globs exactly when this says so, and upload_artifacts
// namespaces exactly when this says so. If the two disagreed, a
// pattern could resolve as a glob but publish under a literal name.
assert!(is_glob("tmp/*.log"));
assert!(is_glob("tmp/**/*.log"));
assert!(is_glob("out-?.txt"));
assert!(is_glob("out-[ab].txt"));
assert!(!is_glob("out.txt"));
assert!(!is_glob("cover/index.html"));
}
// ── resolve_paths: ordinary use ───────────────────────────────────
#[test]
fn a_literal_path_resolves_relative_to_the_workspace() {
let ws = TempDir::new("art-literal");
ws.write("out.txt", "hi");
let found = resolve(&ws, "out.txt");
assert_eq!(file_names(&found), vec!["out.txt"]);
assert!(found.refusals.is_empty(), "{:?}", found.refusals);
}
#[test]
fn resolved_paths_are_canonical_so_the_file_read_is_the_file_checked() {
// The confinement check proves things about the *resolved* path. If
// the unresolved one were returned, upload_single would re-resolve it
// at read time and the check would not cover the bytes sent.
let ws = TempDir::new("art-canonical");
ws.write("real/out.txt", "hi");
std::os::unix::fs::symlink(ws.join("real"), ws.join("link")).unwrap();
let found = resolve(&ws, "link/out.txt");
assert_eq!(found.files.len(), 1, "{:?}", found.files);
assert_eq!(
found.files[0],
found.files[0].canonicalize().unwrap(),
"resolve_paths must return an already-canonical path"
);
assert!(
found.files[0].starts_with(ws.canonical().join("real")),
"expected the resolved target, got {:?}",
found.files[0]
);
}
#[test]
fn a_literal_path_that_does_not_exist_resolves_to_nothing() {
let ws = TempDir::new("art-missing");
ws.write("out.txt", "hi");
assert!(resolve(&ws, "missing.txt").files.is_empty());
}
#[test]
fn a_directory_is_not_collected_as_a_file() {
let ws = TempDir::new("art-dir");
ws.write("cover/index.html", "<html>");
assert!(
resolve(&ws, "cover").files.is_empty(),
"a bare directory has no bytes to upload"
);
}
#[test]
fn a_glob_matches_every_file_and_skips_directories() {
let ws = TempDir::new("art-glob");
ws.write("tmp/a.log", "a");
ws.write("tmp/b.log", "b");
ws.write("tmp/c.txt", "c");
ws.write("tmp/nested/d.log", "d");
let found = resolve(&ws, "tmp/*.log");
assert_eq!(
file_names(&found),
vec!["a.log", "b.log"],
"a single star must not descend into tmp/nested"
);
}
#[test]
fn a_recursive_glob_descends_into_subdirectories() {
let ws = TempDir::new("art-globrec");
ws.write("tmp/a.log", "a");
ws.write("tmp/nested/deep/d.log", "d");
assert_eq!(resolve(&ws, "tmp/**/*.log").files.len(), 2);
}
#[test]
fn a_glob_matching_nothing_resolves_to_nothing() {
let ws = TempDir::new("art-globnone");
ws.write("out.txt", "hi");
assert!(resolve(&ws, "*.nope").files.is_empty());
}
#[test]
fn a_malformed_glob_is_refused_with_a_reason_rather_than_panicking() {
let ws = TempDir::new("art-globbad");
ws.write("out.txt", "hi");
let found = resolve(&ws, "a[");
assert!(found.files.is_empty());
assert!(
found.refusals.iter().any(|r| r.contains("Invalid glob")),
"the author needs to be told their pattern is malformed: {:?}",
found.refusals
);
}
// ── resolve_paths: confinement ────────────────────────────────────
//
// Artifact specs come from the built branch's `.anvil.yml`, so anyone who
// can open a pull request controls these strings.
#[test]
fn an_absolute_path_cannot_reach_outside_the_workspace() {
// `Path::join` REPLACES on an absolute argument, so the naive
// workspace.join("/etc/hostname") is just "/etc/hostname".
let ws = TempDir::new("art-abs");
let outside = TempDir::new("art-abs-out");
let secret = outside.write("secret.txt", "SECRET");
let found = resolve(&ws, secret.to_str().unwrap());
assert!(
found.files.is_empty(),
"an absolute artifact path escaped: {:?}",
found.files
);
assert!(
found
.refusals
.iter()
.any(|r| r.contains("outside the job workspace")),
"the refusal must be explained: {:?}",
found.refusals
);
}
#[test]
fn a_dot_dot_path_cannot_climb_out_of_the_workspace() {
let ws = TempDir::new("art-climb");
let outside = TempDir::new("art-climb-out");
let secret = outside.write("secret.txt", "SECRET");
let rel = format!(
"../{}/secret.txt",
outside.path().file_name().unwrap().to_string_lossy()
);
assert!(secret.exists());
assert!(
resolve(&ws, &rel).files.is_empty(),
"a ../ artifact path escaped the workspace"
);
}
#[test]
fn a_symlink_planted_in_the_workspace_cannot_smuggle_a_file_out() {
// Confinement has to resolve symlinks, or a build step can just
// `ln -s /etc/passwd out.txt` and name out.txt as an artifact.
let ws = TempDir::new("art-link");
let outside = TempDir::new("art-link-out");
let secret = outside.write("secret.txt", "SECRET");
ws.write("keep.txt", "ok");
std::os::unix::fs::symlink(&secret, ws.join("sneaky.txt")).unwrap();
assert!(
resolve(&ws, "sneaky.txt").files.is_empty(),
"a symlink out of the workspace was collected"
);
assert_eq!(
resolve(&ws, "keep.txt").files.len(),
1,
"confinement must not break ordinary files"
);
}
#[test]
fn a_hardlink_to_a_file_outside_the_workspace_is_refused() {
// A hardlink has no link to resolve, so it canonicalizes *inside* the
// workspace and sails through a symlink-only check. Same attacker,
// same result: bytes from outside the tree get published.
let ws = TempDir::new("art-hard");
let outside = TempDir::new("art-hard-out");
let secret = outside.write("secret.txt", "SECRET");
// Same filesystem (both under the temp dir), so the link is allowed
// to be created — which is exactly the situation being defended.
std::fs::hard_link(&secret, ws.join("out.txt")).expect("hard link within one filesystem");
let found = resolve(&ws, "out.txt");
assert!(
found.files.is_empty(),
"a hardlinked outside file was collected: {:?}",
found.files
);
assert!(
found.refusals.iter().any(|r| r.contains("hard link")),
"the refusal must name the reason so the author can copy instead: {:?}",
found.refusals
);
}
#[test]
fn a_sibling_directory_sharing_the_workspaces_name_prefix_is_not_inside_it() {
// `/work-evil` must not count as inside `/work`. Rust's
// `Path::starts_with` is component-wise, which is what makes this
// safe — a naive string prefix check would not be.
let base = TempDir::new("art-prefix");
let ws = base.join("work");
std::fs::create_dir_all(&ws).unwrap();
let evil = base.join("work-evil");
std::fs::create_dir_all(&evil).unwrap();
std::fs::write(evil.join("secret.txt"), "SECRET").unwrap();
let root = ws.canonicalize().unwrap();
let found = resolve_paths(&root, evil.join("secret.txt").to_str().unwrap());
assert!(
found.files.is_empty(),
"a sibling with a shared name prefix was treated as inside: {:?}",
found.files
);
}
#[test]
fn a_glob_still_collects_files_reached_through_an_in_workspace_symlinked_dir() {
// Confinement resolves symlinks, so a legitimate symlinked build
// directory *inside* the workspace must still be collected.
let ws = TempDir::new("art-linkdir");
ws.write("real/out.log", "data");
std::os::unix::fs::symlink(ws.join("real"), ws.join("link")).unwrap();
assert_eq!(resolve(&ws, "link/*.log").files.len(), 1);
}
// ── upload_artifacts ──────────────────────────────────────────────
fn config_for(server: &MockServer) -> RunnerConfig {
RunnerConfig::for_test(&server.uri(), "http://ollama.invalid")
}
async fn accepting_server() -> MockServer {
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(201))
.mount(&server)
.await;
server
}
#[tokio::test]
async fn no_specs_means_no_requests_and_no_uploads() {
let server = MockServer::start().await;
let ws = TempDir::new("art-nospec");
ws.write("out.txt", "hi");
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &[]).await;
assert_eq!(out.uploaded, 0);
assert!(
server.received_requests().await.unwrap().is_empty(),
"an empty spec list must not touch the network"
);
}
#[tokio::test]
async fn an_unresolvable_workspace_uploads_nothing_and_says_why() {
let server = MockServer::start().await;
let missing = std::env::temp_dir().join("anvil-artifacts-definitely-absent");
let specs = vec![ArtifactSpec {
name: "x".into(),
path: "out.txt".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", &missing, &specs).await;
assert_eq!(out.uploaded, 0);
assert!(
out.notices.iter().any(|n| n.contains("does not resolve")),
"{:?}",
out.notices
);
}
#[tokio::test]
async fn a_single_match_uploads_under_the_spec_name_to_the_job_endpoint() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(req_path("/api/v1/runners/jobs/job-42/artifacts"))
.respond_with(ResponseTemplate::new(201))
.expect(1)
.mount(&server)
.await;
let ws = TempDir::new("art-single");
ws.write("out.txt", "hi");
let specs = vec![ArtifactSpec {
name: "report".into(),
path: "out.txt".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-42", ws.path(), &specs).await;
assert_eq!(out.uploaded, 1);
let reqs = server.received_requests().await.unwrap();
let body = String::from_utf8_lossy(&reqs[0].body);
assert!(
body.contains("report"),
"the multipart body must carry the artifact name, got: {body}"
);
assert_eq!(
reqs[0].headers.get("authorization").unwrap(),
"Bearer rt_secret",
"artifact upload must authenticate as the runner"
);
}
#[tokio::test]
async fn several_matches_are_namespaced_by_relative_path_so_they_do_not_collide() {
let server = accepting_server().await;
let ws = TempDir::new("art-multi");
ws.write("tmp/a.log", "a");
ws.write("tmp/b.log", "b");
let specs = vec![ArtifactSpec {
name: "logs".into(),
path: "tmp/*.log".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 2);
let joined: String = server
.received_requests()
.await
.unwrap()
.iter()
.map(|r| String::from_utf8_lossy(&r.body).to_string())
.collect::<Vec<_>>()
.join("\n");
assert!(
joined.contains("logs/tmp/a.log") && joined.contains("logs/tmp/b.log"),
"multi-file artifacts must keep distinct names, got: {joined}"
);
}
#[tokio::test]
async fn a_glob_that_matches_one_file_still_publishes_the_namespaced_name() {
// The name comes from the spec's shape, not the surviving count.
// Deriving it from `files.len() > 1` meant that dropping a sibling —
// a symlink refused by confinement, or a file deleted mid-job —
// silently renamed the survivor from `logs/tmp/a.log` to `logs`, and
// any downstream fetch by the documented name 404s.
let server = accepting_server().await;
let ws = TempDir::new("art-onematch");
ws.write("tmp/a.log", "a");
let specs = vec![ArtifactSpec {
name: "logs".into(),
path: "tmp/*.log".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 1);
let body =
String::from_utf8_lossy(&server.received_requests().await.unwrap()[0].body).to_string();
assert!(
body.contains("logs/tmp/a.log"),
"a glob must publish a namespaced name even when it matches once: {body}"
);
}
#[tokio::test]
async fn refusing_one_match_does_not_rename_its_surviving_sibling() {
let server = accepting_server().await;
let ws = TempDir::new("art-sibling");
let outside = TempDir::new("art-sibling-out");
let secret = outside.write("secret", "SECRET");
ws.write("tmp/a.log", "a");
std::os::unix::fs::symlink(&secret, ws.join("tmp/b.log")).unwrap();
let specs = vec![ArtifactSpec {
name: "logs".into(),
path: "tmp/*.log".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 1, "only the legitimate file uploads");
let body =
String::from_utf8_lossy(&server.received_requests().await.unwrap()[0].body).to_string();
assert!(
body.contains("logs/tmp/a.log"),
"the survivor keeps its name, got: {body}"
);
}
#[tokio::test]
async fn a_spec_matching_nothing_is_reported_and_does_not_fail_the_others() {
let server = accepting_server().await;
let ws = TempDir::new("art-ghost");
ws.write("out.txt", "hi");
let specs = vec![
ArtifactSpec {
name: "ghost".into(),
path: "never-written.txt".into(),
},
ArtifactSpec {
name: "real".into(),
path: "out.txt".into(),
},
];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 1, "the surviving spec must still upload");
assert!(
out.notices
.iter()
.any(|n| n.contains("never-written.txt") && n.contains("ghost")),
"the empty spec must be named in the job log: {:?}",
out.notices
);
}
#[tokio::test]
async fn a_refusal_is_surfaced_as_a_notice_for_the_job_log() {
// Explaining the refusal only on the runner host's stderr leaves the
// pipeline author with artifacts that silently stopped appearing.
let server = accepting_server().await;
let ws = TempDir::new("art-notice");
let outside = TempDir::new("art-notice-out");
let secret = outside.write("secret.txt", "SECRET");
let specs = vec![ArtifactSpec {
name: "secrets".into(),
path: secret.to_string_lossy().to_string(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 0);
assert!(
out.notices
.iter()
.any(|n| n.contains("Refusing") && n.contains("outside the job workspace")),
"the reason must reach the caller, got: {:?}",
out.notices
);
}
#[tokio::test]
async fn a_rejected_upload_is_not_counted_and_is_reported() {
// The count is what the operator sees; reporting a success the
// server refused would hide a broken artifact pipeline entirely.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(413).set_body_string("too large"))
.mount(&server)
.await;
let ws = TempDir::new("art-rejected");
ws.write("out.txt", "hi");
let specs = vec![ArtifactSpec {
name: "report".into(),
path: "out.txt".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(out.uploaded, 0);
assert!(
out.notices.iter().any(|n| n.contains("Failed to upload")),
"{:?}",
out.notices
);
}
#[tokio::test]
async fn one_failed_upload_does_not_abandon_the_remaining_artifacts() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(500))
.up_to_n_times(1)
.expect(1)
.mount(&server)
.await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(201))
.mount(&server)
.await;
let ws = TempDir::new("art-partial");
ws.write("tmp/a.log", "a");
ws.write("tmp/b.log", "b");
let specs = vec![ArtifactSpec {
name: "logs".into(),
path: "tmp/*.log".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(
out.uploaded, 1,
"the second artifact must still be attempted"
);
}
#[tokio::test]
async fn a_file_over_the_size_cap_is_refused_before_it_is_sent() {
// The cap exists to protect the server; checking it after the upload
// would defeat the point.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(201))
.expect(0)
.mount(&server)
.await;
let ws = TempDir::new("art-toobig");
let big = ws.join("big.bin");
let f = std::fs::File::create(&big).unwrap();
f.set_len(MAX_ARTIFACT_SIZE + 1).unwrap();
drop(f);
let specs = vec![ArtifactSpec {
name: "big".into(),
path: "big.bin".into(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(
out.uploaded, 0,
"an oversized artifact must not be uploaded"
);
}
#[tokio::test]
async fn upload_single_refuses_a_symlink_that_appears_after_resolution() {
// The last line of defence for the resolve/read window: even handed a
// path that has since become a symlink, the upload must not follow it.
let server = accepting_server().await;
let ws = TempDir::new("art-nofollow");
let outside = TempDir::new("art-nofollow-out");
let secret = outside.write("secret.txt", "SECRET");
let swapped = ws.join("out.txt");
std::os::unix::fs::symlink(&secret, &swapped).unwrap();
let err = upload_single(
&reqwest::Client::new(),
&config_for(&server),
"job-1",
&swapped,
"report",
)
.await
.expect_err("a symlinked final component must not be followed");
assert!(
server.received_requests().await.unwrap().is_empty(),
"nothing may be sent: {err}"
);
}
#[tokio::test]
async fn an_absolute_spec_path_uploads_nothing() {
// End-to-end companion to the resolve_paths confinement tests: the
// escape must be closed on the path the runner actually walks.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(201))
.expect(0)
.mount(&server)
.await;
let ws = TempDir::new("art-e2e");
ws.write("out.txt", "hi");
let outside = TempDir::new("art-e2e-out");
let secret = outside.write("secret.txt", "SECRET");
let specs = vec![ArtifactSpec {
name: "secrets".into(),
path: secret.to_string_lossy().to_string(),
}];
let out = upload_artifacts(&config_for(&server), "job-1", ws.path(), &specs).await;
assert_eq!(
out.uploaded, 0,
"a file outside the workspace must never be uploaded"
);
}
}
src/runner/config.rs +483 −3
@@ -63,14 +63,51 @@
Ok(serde_json::from_str(&contents)?)
}
/// Persist the config, preferring an atomic private write and falling back
/// to writing the file in place.
///
/// The fallback is not belt-and-braces, it is the deployments that only
/// ever worked that way. A config bind-mounted as a single file (`-v
/// /host/runner.json:/root/.anvil-runner/config.json`, a Kubernetes
/// secret or configMap file mount) cannot be replaced by `rename` — the
/// mount point is busy — and a config in a root-owned directory cannot
/// have a sibling temp file created next to it. The plain `fs::write` this
/// replaced truncated the existing inode and succeeded in both cases.
///
/// Losing that would be worse than it sounds: `runner configure` saves the
/// `runner_token` the server issues exactly once, so a failed save leaves
/// a registered runner whose credential no longer exists anywhere.
pub fn save(&self, path: Option<&str>) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
let p = Self::path(path);
if let Some(parent) = p.parent() {
// Best-effort: when the config is a mounted file the parent may
// not be writable, which is survivable as long as the file exists.
std::fs::create_dir_all(parent)?;
let _ = std::fs::create_dir_all(parent);
}
let contents = serde_json::to_string_pretty(self)?;
match write_private(&p, contents.as_bytes()) {
Ok(()) => Ok(()),
Err(atomic_err) => match write_in_place(&p, contents.as_bytes()) {
Ok(()) => {
eprintln!(
"note: {} could not be replaced atomically ({atomic_err}); \
wrote it in place instead",
p.display()
);
Ok(())
}
// Report both: the in-place error is usually the informative
// one, but on a read-only mount the atomic error explains why
// the preferred path was abandoned.
Err(in_place_err) => Err(format!(
"could not write {}: {in_place_err} \
(atomic replace also failed: {atomic_err})",
p.display()
)
.into()),
std::fs::write(&p, contents)?;
Ok(())
},
}
}
pub fn delete(path: Option<&str>) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
@@ -92,5 +129,448 @@
pub fn work_dir_path(&self) -> PathBuf {
PathBuf::from(&self.work_dir)
}
}
/// Write the runner config the way [`crate::config`] writes the user config:
/// into a fresh temp file created at mode 0600, then renamed over the target.
///
/// This file holds `runner_token` — a credential that can claim and report CI
/// jobs — so it must never exist world-readable, not even briefly. Creating
/// with the mode (rather than chmod-ing afterwards) closes that window, and
/// because `rename` carries the temp file's mode across, a config left at 0644
/// by an older build is *replaced* rather than written into. The rename also
/// makes the swap atomic, so an interrupted save can't leave a truncated
/// config that `load` then rejects.
fn write_private(
path: &std::path::Path,
contents: &[u8],
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
use std::io::Write;
let dir = path.parent().unwrap_or_else(|| std::path::Path::new("."));
let stem = path
.file_name()
.and_then(|s| s.to_str())
.unwrap_or("config.json");
let nanos = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_nanos())
.unwrap_or(0);
let tmp = dir.join(format!(".{stem}.{}.{nanos}.tmp", std::process::id()));
let written = (|| -> Result<(), std::io::Error> {
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create_new(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut file = opts.open(&tmp)?;
file.write_all(contents)?;
file.sync_all()
})();
if let Err(e) = written {
let _ = std::fs::remove_file(&tmp);
return Err(e.into());
}
if let Err(e) = std::fs::rename(&tmp, path) {
let _ = std::fs::remove_file(&tmp);
return Err(e.into());
}
Ok(())
}
/// Write the config into the existing file, truncating it.
///
/// The fallback for the deployments [`RunnerConfig::save`] documents. It gives
/// up atomicity — an interrupted write here leaves a truncated config — but a
/// config that cannot be saved at all is the worse failure.
///
/// The 0600 mode still applies when this call *creates* the file; when the
/// file already exists the mode is ignored by `open`, so the permissions are
/// tightened separately afterwards. That last step is best-effort on purpose:
/// on a bind-mounted file we may not own the inode, and on DrvFs/NTFS/CIFS
/// `set_permissions` is simply unsupported — in both cases failing the save
/// over the mode would defeat the point of having a fallback.
fn write_in_place(
path: &std::path::Path,
contents: &[u8],
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
use std::io::Write;
let mut opts = std::fs::OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
opts.mode(0o600);
}
let mut file = opts.open(path)?;
file.write_all(contents)?;
file.sync_all()?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
}
Ok(())
}
impl RunnerConfig {
/// Minimal config for tests: only the fields a unit test actually varies.
#[cfg(test)]
pub fn for_test(server_url: &str, ollama_url: &str) -> Self {
Self {
server_url: server_url.to_string(),
runner_id: "runner-1".into(),
runner_token: "rt_secret".into(),
name: "test-runner".into(),
labels: vec!["linux".into()],
work_dir: "/tmp/anvil-runner".into(),
parallel: 1,
poll_interval_ms: default_poll_interval(),
heartbeat_interval_ms: default_heartbeat_interval(),
once: false,
ephemeral: false,
cleanup: default_cleanup(),
ollama_url: ollama_url.to_string(),
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::testutil::TempDir;
#[test]
fn save_then_load_round_trips_every_field() {
// A spot-check on one field would pass while a rename in the serde
// attributes silently dropped the rest.
let dir = TempDir::new("cfg-roundtrip");
let p = dir.join("config.json");
let path = p.to_str().unwrap();
let mut original = RunnerConfig::for_test("https://anvil.test", "http://localhost:11434");
original.labels = vec!["linux".into(), "gpu".into()];
original.parallel = 4;
original.ephemeral = true;
original.save(Some(path)).unwrap();
let loaded = RunnerConfig::load(Some(path)).unwrap();
assert_eq!(loaded.server_url, original.server_url);
assert_eq!(loaded.runner_id, original.runner_id);
assert_eq!(loaded.runner_token, original.runner_token);
assert_eq!(loaded.name, original.name);
assert_eq!(loaded.labels, original.labels);
assert_eq!(loaded.work_dir, original.work_dir);
assert_eq!(loaded.parallel, original.parallel);
assert_eq!(loaded.poll_interval_ms, original.poll_interval_ms);
assert_eq!(loaded.heartbeat_interval_ms, original.heartbeat_interval_ms);
assert_eq!(loaded.once, original.once);
assert_eq!(loaded.ephemeral, original.ephemeral);
assert_eq!(loaded.cleanup, original.cleanup);
assert_eq!(loaded.ollama_url, original.ollama_url);
}
#[cfg(unix)]
fn mode_of(p: &std::path::Path) -> u32 {
use std::os::unix::fs::PermissionsExt;
std::fs::metadata(p).unwrap().permissions().mode() & 0o777
}
#[cfg(unix)]
#[test]
fn save_never_leaves_the_runner_token_world_readable() {
// The file holds a credential that can claim and report CI jobs. The
// user config (`crate::config`) has always been written 0600; this one
// was written with a plain `fs::write`, i.e. 0644 under a default
// umask — readable by every account on a shared runner host.
let dir = TempDir::new("cfg-mode");
let p = dir.join("config.json");
RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some(p.to_str().unwrap()))
.unwrap();
assert_eq!(
mode_of(&p),
0o600,
"runner config holds runner_token; got {:o}",
mode_of(&p)
);
}
#[cfg(unix)]
#[test]
fn save_tightens_the_mode_of_a_config_left_permissive_by_an_older_build() {
// Upgrading must fix an existing 0644 file, not write into it.
use std::os::unix::fs::PermissionsExt;
let dir = TempDir::new("cfg-tighten");
let p = dir.join("config.json");
std::fs::write(&p, b"{}").unwrap();
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o644)).unwrap();
RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some(p.to_str().unwrap()))
.unwrap();
assert_eq!(mode_of(&p), 0o600, "expected the save to replace the file");
}
#[test]
fn save_creates_missing_parent_directories() {
let dir = TempDir::new("cfg-mkdir");
let p = dir.join("nested/deeper/config.json");
RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some(p.to_str().unwrap()))
.unwrap();
assert!(p.exists(), "save must mkdir -p its parent");
}
#[test]
fn save_leaves_no_temp_file_behind() {
// The atomic write uses a dotfile temp; a leaked one would be a
// second, world-visible copy of the token.
let dir = TempDir::new("cfg-notemp");
let p = dir.join("config.json");
RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some(p.to_str().unwrap()))
.unwrap();
let leftovers: Vec<String> = std::fs::read_dir(dir.path())
.unwrap()
.filter_map(Result::ok)
.map(|e| e.file_name().to_string_lossy().to_string())
.filter(|n| n != "config.json")
.collect();
assert!(
leftovers.is_empty(),
"temp files left behind: {leftovers:?}"
);
}
// ── the in-place fallback ─────────────────────────────────────────
//
// `runner configure` persists a `runner_token` the server issues exactly
// once. A save that fails leaves a registered runner whose credential
// exists nowhere, so the deployments that can only be written in place
// matter more than the atomicity they give up.
#[cfg(unix)]
#[test]
fn save_falls_back_to_writing_in_place_when_the_directory_forbids_a_temp_file() {
// Stands in for a config in a root-owned directory, or one bind-mounted
// as a single file: the existing inode is writable, but no sibling can
// be created next to it, so the temp-then-rename cannot work.
use std::os::unix::fs::PermissionsExt;
if crate::testutil::running_as_root() {
eprintln!("skipped: root bypasses directory write permissions");
return;
}
let dir = TempDir::new("cfg-inplace");
let p = dir.join("config.json");
std::fs::write(&p, b"{}").unwrap();
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o600)).unwrap();
// r-x: existing files stay writable, new entries cannot be created.
std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o500)).unwrap();
let mut cfg = RunnerConfig::for_test("https://anvil.test", "http://x");
cfg.runner_token = "rt_only_issued_once".into();
let result = cfg.save(Some(p.to_str().unwrap()));
// Restore before asserting, so a failure cannot leave an
// undeletable directory behind for TempDir's Drop.
std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o700)).unwrap();
result.expect("a config that can only be written in place must still save");
let loaded = RunnerConfig::load(Some(p.to_str().unwrap())).unwrap();
assert_eq!(
loaded.runner_token, "rt_only_issued_once",
"the token the server issued once must survive the fallback"
);
}
#[cfg(unix)]
#[test]
fn the_in_place_write_creates_the_file_private() {
let dir = TempDir::new("cfg-inplace-mode");
let p = dir.join("config.json");
write_in_place(&p, b"{}").unwrap();
assert_eq!(mode_of(&p), 0o600);
}
#[cfg(unix)]
#[test]
fn the_in_place_write_tightens_a_file_it_did_not_create() {
use std::os::unix::fs::PermissionsExt;
let dir = TempDir::new("cfg-inplace-tighten");
let p = dir.join("config.json");
std::fs::write(&p, b"old").unwrap();
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o644)).unwrap();
write_in_place(&p, b"{}").unwrap();
assert_eq!(
mode_of(&p),
0o600,
"mode only applies on create, so an existing file must be chmod'd"
);
}
#[test]
fn a_save_that_cannot_be_written_at_all_reports_both_attempts() {
// The operator needs to see why the preferred path was abandoned as
// well as why the fallback failed.
let err = RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some("/proc/definitely/not/writable/config.json"))
.unwrap_err()
.to_string();
assert!(
err.contains("atomic replace also failed"),
"both failures must be reported, got: {err}"
);
}
// ── load ──────────────────────────────────────────────────────────
#[test]
fn load_applies_defaults_for_fields_an_older_config_omits() {
// A config written before the poll/heartbeat/ollama fields existed
// must still load — otherwise an upgrade bricks every runner.
let dir = TempDir::new("cfg-defaults");
let p = dir.write(
"config.json",
r#"{"server_url":"https://a.test","runner_id":"r","runner_token":"t",
"name":"n","labels":[],"work_dir":"/w","parallel":2}"#,
);
let loaded = RunnerConfig::load(Some(p.to_str().unwrap())).unwrap();
assert_eq!(loaded.poll_interval_ms, 5000);
assert_eq!(loaded.heartbeat_interval_ms, 30000);
assert_eq!(loaded.cleanup, "never");
assert_eq!(loaded.ollama_url, "http://localhost:11434");
assert!(!loaded.once);
assert!(!loaded.ephemeral);
}
#[test]
fn load_of_a_missing_config_names_the_path_and_the_fix() {
// This message is the entire onboarding experience for a new runner.
let err = RunnerConfig::load(Some("/nonexistent/anvil/config.json"))
.unwrap_err()
.to_string();
assert!(
err.contains("/nonexistent/anvil/config.json"),
"the error must name the path it looked at, got: {err}"
);
assert!(
err.contains("anvil runner configure"),
"the error must name the command that fixes it, got: {err}"
);
}
#[test]
fn load_of_a_corrupt_config_fails_rather_than_yielding_defaults() {
let dir = TempDir::new("cfg-corrupt");
let p = dir.write("config.json", "{ not json");
assert!(
RunnerConfig::load(Some(p.to_str().unwrap())).is_err(),
"a truncated config must not silently become a default runner"
);
}
#[test]
fn delete_removes_the_config_and_is_idempotent() {
let dir = TempDir::new("cfg-delete");
let p = dir.join("config.json");
let path = p.to_str().unwrap();
RunnerConfig::for_test("https://anvil.test", "http://x")
.save(Some(path))
.unwrap();
RunnerConfig::delete(Some(path)).unwrap();
assert!(!p.exists(), "delete must remove the file");
RunnerConfig::delete(Some(path)).expect("deleting an absent config must not error");
}
// ── paths and URLs ────────────────────────────────────────────────
#[test]
fn path_prefers_an_explicit_override_over_the_home_default() {
assert_eq!(
RunnerConfig::path(Some("/etc/anvil/runner.json")),
PathBuf::from("/etc/anvil/runner.json")
);
}
#[test]
fn the_default_path_lives_under_a_dot_anvil_runner_directory() {
let p = RunnerConfig::path(None);
assert_eq!(p.file_name().unwrap(), "config.json");
assert_eq!(p.parent().unwrap().file_name().unwrap(), ".anvil-runner");
}
#[test]
fn api_url_inserts_the_version_prefix_exactly_once() {
let c = RunnerConfig::for_test("https://anvil.test", "http://x");
assert_eq!(
c.api_url("/runners/abc/heartbeat"),
"https://anvil.test/api/v1/runners/abc/heartbeat"
);
}
#[test]
fn api_url_does_not_double_the_slash_on_a_trailing_slash_server_url() {
// `anvil runner configure` happily accepts a pasted URL with a
// trailing slash; a `//api/v1` path 404s on some proxies.
let c = RunnerConfig::for_test("https://anvil.test/", "http://x");
assert_eq!(
c.api_url("/runners/x"),
"https://anvil.test/api/v1/runners/x"
);
}
#[test]
fn api_url_collapses_several_trailing_slashes() {
let c = RunnerConfig::for_test("https://anvil.test///", "http://x");
assert_eq!(c.api_url("/x"), "https://anvil.test/api/v1/x");
}
#[test]
fn auth_header_is_a_bearer_token() {
let c = RunnerConfig::for_test("https://anvil.test", "http://x");
assert_eq!(c.auth_header(), "Bearer rt_secret");
}
#[test]
fn work_dir_path_reflects_the_configured_work_dir() {
let mut c = RunnerConfig::for_test("https://anvil.test", "http://x");
c.work_dir = "/var/lib/anvil/work".into();
assert_eq!(c.work_dir_path(), PathBuf::from("/var/lib/anvil/work"));
}
}
src/runner/heartbeat.rs +377 −0
@@ -140,3 +140,380 @@
}
}
#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;
use wiremock::matchers::{method, path as req_path};
use wiremock::{Mock, MockServer, ResponseTemplate};
/// An Ollama that answers `/api/tags` with `body`.
async fn ollama_serving(body: serde_json::Value) -> MockServer {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/tags"))
.respond_with(ResponseTemplate::new(200).set_body_json(body))
.mount(&server)
.await;
server
}
/// A URL whose port is closed, so `connect` is refused immediately.
///
/// Binding then dropping a listener hands back a port the OS has just
/// confirmed free. Hard-coding something like `127.0.0.1:1` instead is a
/// portability trap: on WSL2 that address is black-holed rather than
/// refused, so the connect blocks until its timeout and a test that should
/// take a millisecond takes thirty seconds.
fn closed_port_url() -> String {
let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind an ephemeral port");
let port = listener.local_addr().unwrap().port();
drop(listener);
format!("http://127.0.0.1:{port}")
}
/// An Ollama that refuses immediately. Preferred over an unroutable
/// address: a closed port is not refused quickly on every platform (WSL2
/// black-holes it), so pointing at one costs the probe's full 3s timeout
/// and makes the test look hung.
async fn ollama_refusing() -> MockServer {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/tags"))
.respond_with(ResponseTemplate::new(500))
.mount(&server)
.await;
server
}
/// Poll until the mock has seen at least `n` requests, rather than
/// sleeping a fixed interval and hoping. Fails loudly instead of hanging.
///
/// The budget has to clear `local_models`' 3s timeout, which every
/// heartbeat pays before it posts when Ollama is unreachable.
async fn wait_for_requests(server: &MockServer, n: usize) -> Vec<wiremock::Request> {
for _ in 0..1000 {
if let Some(reqs) = server.received_requests().await {
if reqs.len() >= n {
return reqs;
}
}
tokio::time::sleep(std::time::Duration::from_millis(10)).await;
}
let seen = server
.received_requests()
.await
.map(|r| r.len())
.unwrap_or(0);
panic!("wanted {n} heartbeat(s) within 10s, saw {seen}");
}
async fn first_request(server: &MockServer) -> wiremock::Request {
wait_for_requests(server, 1).await.remove(0)
}
// ── local_models ──────────────────────────────────────────────────
#[tokio::test]
async fn models_are_reported_with_the_capabilities_ollama_declares() {
// The server filters the chat model picker on these: an agent chat
// always sends tools, so a model advertised without `tools` would be
// offered only to fail on its first call.
let ollama = ollama_serving(json!({"models": [
{"name": "llama3.2:3b", "capabilities": ["completion", "tools"]}
]}))
.await;
let models = local_models(&reqwest::Client::new(), &ollama.uri())
.await
.expect("a served model must be reported");
assert_eq!(
models,
vec![json!({"name": "llama3.2:3b", "capabilities": ["completion", "tools"]})]
);
}
#[tokio::test]
async fn a_model_with_no_capabilities_omits_the_key_rather_than_claiming_none() {
// The server reads a missing key as *unknown* and keeps the model; an
// empty list would read as "supports nothing" and hide it forever.
let ollama = ollama_serving(json!({"models": [{"name": "mystery:latest"}]})).await;
let models = local_models(&reqwest::Client::new(), &ollama.uri())
.await
.expect("the model is still served");
assert_eq!(models, vec![json!({"name": "mystery:latest"})]);
assert!(
models[0].get("capabilities").is_none(),
"unknown capabilities must be absent, not empty: {}",
models[0]
);
}
#[tokio::test]
async fn an_empty_capabilities_array_is_also_reported_as_unknown() {
let ollama = ollama_serving(json!({"models": [{"name": "m", "capabilities": []}]})).await;
let models = local_models(&reqwest::Client::new(), &ollama.uri())
.await
.unwrap();
assert!(
models[0].get("capabilities").is_none(),
"got: {}",
models[0]
);
}
#[tokio::test]
async fn non_string_capabilities_are_dropped_without_dropping_the_model() {
let ollama = ollama_serving(json!({"models": [
{"name": "m", "capabilities": ["tools", 7, null]}
]}))
.await;
let models = local_models(&reqwest::Client::new(), &ollama.uri())
.await
.unwrap();
assert_eq!(models[0]["capabilities"], json!(["tools"]));
}
#[tokio::test]
async fn a_model_without_a_name_is_skipped_and_its_siblings_survive() {
let ollama = ollama_serving(json!({"models": [
{"capabilities": ["tools"]},
{"name": "good:1b"}
]}))
.await;
let models = local_models(&reqwest::Client::new(), &ollama.uri())
.await
.unwrap();
assert_eq!(models, vec![json!({"name": "good:1b"})]);
}
#[tokio::test]
async fn an_ollama_serving_nothing_reports_none_so_the_prior_list_stands() {
// Reporting an empty list would *retract* the models this runner
// advertised a moment ago; omitting the key leaves them in place.
let ollama = ollama_serving(json!({"models": []})).await;
assert!(local_models(&reqwest::Client::new(), &ollama.uri())
.await
.is_none());
}
#[tokio::test]
async fn a_response_without_a_models_key_reports_none() {
let ollama = ollama_serving(json!({"something_else": true})).await;
assert!(local_models(&reqwest::Client::new(), &ollama.uri())
.await
.is_none());
}
#[tokio::test]
async fn an_ollama_error_status_reports_none_rather_than_propagating() {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/tags"))
.respond_with(ResponseTemplate::new(500))
.mount(&server)
.await;
assert!(local_models(&reqwest::Client::new(), &server.uri())
.await
.is_none());
}
#[tokio::test]
async fn a_non_json_body_reports_none_rather_than_panicking() {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(req_path("/api/tags"))
.respond_with(ResponseTemplate::new(200).set_body_string("<html>nope</html>"))
.mount(&server)
.await;
assert!(local_models(&reqwest::Client::new(), &server.uri())
.await
.is_none());
}
#[tokio::test]
async fn an_unreachable_ollama_reports_none_rather_than_costing_a_heartbeat() {
// Best-effort is the whole contract here: a dead Ollama must not
// stop the runner reporting that it is alive.
assert!(local_models(&reqwest::Client::new(), &closed_port_url())
.await
.is_none());
}
#[tokio::test]
async fn a_trailing_slash_on_the_ollama_url_does_not_double_the_path() {
// `anvil runner configure` accepts a pasted URL; //api/tags 404s.
let ollama = ollama_serving(json!({"models": [{"name": "m"}]})).await;
let with_slash = format!("{}/", ollama.uri());
assert!(
local_models(&reqwest::Client::new(), &with_slash)
.await
.is_some(),
"the mock only answers /api/tags, so a doubled slash means None"
);
}
// ── send_once ─────────────────────────────────────────────────────
#[tokio::test]
async fn send_once_posts_to_the_runners_own_heartbeat_endpoint_with_its_token() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(req_path("/api/v1/runners/runner-1/heartbeat"))
.respond_with(ResponseTemplate::new(200))
.expect(1)
.mount(&server)
.await;
let config = RunnerConfig::for_test(&server.uri(), &closed_port_url());
send_once(&config).await.expect("a 200 is a success");
let req = first_request(&server).await;
assert_eq!(
req.headers.get("authorization").unwrap(),
"Bearer rt_secret"
);
}
#[tokio::test]
async fn send_once_reports_a_rejected_heartbeat_with_its_status() {
// `runner start` uses this to validate credentials before daemonising;
// swallowing a 401 would let a runner start with a dead token.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(401))
.mount(&server)
.await;
let config = RunnerConfig::for_test(&server.uri(), &closed_port_url());
let err = send_once(&config).await.unwrap_err().to_string();
assert!(
err.contains("401"),
"the error must carry the status so the operator can tell auth from outage, got: {err}"
);
}
#[tokio::test]
async fn send_once_errors_when_the_server_cannot_be_reached() {
let closed = closed_port_url();
let mut config = RunnerConfig::for_test(&closed, &closed);
config.runner_id = "runner-1".into();
assert!(send_once(&config).await.is_err());
}
// ── start: the periodic payload ───────────────────────────────────
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn the_heartbeat_payload_carries_capacity_telemetry() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(req_path("/api/v1/runners/runner-1/heartbeat"))
.respond_with(ResponseTemplate::new(200))
.mount(&server)
.await;
let ollama = ollama_refusing().await;
let mut config = RunnerConfig::for_test(&server.uri(), &ollama.uri());
config.parallel = 4;
let slots = SlotCounter::new();
let _busy = slots.reserve();
let _also_busy = slots.reserve();
let handle = start(&config, slots.clone());
let req = first_request(&server).await;
handle.abort();
let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap();
assert_eq!(body["parallel"], json!(4), "body was: {body}");
assert_eq!(
body["slots_busy"],
json!(2),
"the server schedules on this; a stale count oversubscribes the runner: {body}"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn a_heartbeat_omits_the_metadata_key_when_ollama_is_unreachable() {
// Sending `inference_models: []` would retract the models a peer
// runner reported; the key must simply be absent.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(200))
.mount(&server)
.await;
let ollama = ollama_refusing().await;
let config = RunnerConfig::for_test(&server.uri(), &ollama.uri());
let handle = start(&config, SlotCounter::new());
let req = first_request(&server).await;
handle.abort();
let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap();
assert!(
body.get("metadata").is_none(),
"expected no metadata key, got: {body}"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn a_heartbeat_advertises_the_local_models_when_ollama_answers() {
let ollama = ollama_serving(json!({"models": [
{"name": "llama3.2:3b", "capabilities": ["tools"]}
]}))
.await;
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(200))
.mount(&server)
.await;
let config = RunnerConfig::for_test(&server.uri(), &ollama.uri());
let handle = start(&config, SlotCounter::new());
let req = first_request(&server).await;
handle.abort();
let body: serde_json::Value = serde_json::from_slice(&req.body).unwrap();
assert_eq!(
body["metadata"]["inference_models"],
json!([{"name": "llama3.2:3b", "capabilities": ["tools"]}]),
"body was: {body}"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn a_server_that_rejects_the_heartbeat_does_not_stop_the_loop() {
// A runner that gives up heartbeating on one 500 disappears from the
// dashboard until it is restarted by hand.
let server = MockServer::start().await;
Mock::given(method("POST"))
.respond_with(ResponseTemplate::new(500))
.mount(&server)
.await;
let ollama = ollama_refusing().await;
let mut config = RunnerConfig::for_test(&server.uri(), &ollama.uri());
config.heartbeat_interval_ms = 20;
let handle = start(&config, SlotCounter::new());
// wait_for_requests panics with a diagnosis if the loop died.
let seen = wait_for_requests(&server, 2).await.len();
handle.abort();
assert!(seen >= 2, "saw only {seen} heartbeat(s)");
}
}
src/runner/inference.rs +482 −0
@@ -380,3 +380,485 @@
}
}
#[cfg(test)]
mod translation_tests {
//! The relay's whole job is translating between two wire formats it does
//! not control. Nothing downstream type-checks these shapes: a wrong key
//! or a dropped block does not fail loudly, it produces a model call that
//! silently ignores half the conversation. These tests pin the shape.
use super::*;
// ── Anvil → Ollama: messages ──────────────────────────────────────
#[test]
fn the_system_option_is_prepended_as_a_system_message() {
let req = json!({"messages": [{"role": "user", "content": "hi"}]});
let options = json!({"system": "You are terse."});
let msgs = ollama_messages(&req, &options);
assert_eq!(
msgs[0],
json!({"role": "system", "content": "You are terse."}),
"the system prompt must lead, or the model reads it as a user turn"
);
assert_eq!(msgs[1], json!({"role": "user", "content": "hi"}));
assert_eq!(msgs.len(), 2);
}
#[test]
fn no_system_option_means_no_system_message() {
let req = json!({"messages": [{"role": "user", "content": "hi"}]});
let msgs = ollama_messages(&req, &json!({}));
assert_eq!(
msgs.len(),
1,
"got an unexpected synthetic message: {msgs:?}"
);
assert_eq!(msgs[0]["role"], "user");
}
#[test]
fn a_request_with_no_messages_yields_no_messages() {
assert!(ollama_messages(&json!({}), &json!({})).is_empty());
}
#[test]
fn a_string_content_message_keeps_its_role_and_text() {
let out = translate_message(&json!({"role": "assistant", "content": "sure"}));
assert_eq!(out, vec![json!({"role": "assistant", "content": "sure"})]);
}
#[test]
fn a_message_with_no_content_becomes_empty_text_rather_than_vanishing() {
// Dropping it would silently renumber the turn order the model sees.
let out = translate_message(&json!({"role": "user"}));
assert_eq!(out, vec![json!({"role": "user", "content": ""})]);
}
#[test]
fn a_message_with_no_role_defaults_to_user() {
let out = translate_message(&json!({"content": "orphan"}));
assert_eq!(out[0]["role"], "user");
}
// ── Anvil → Ollama: content blocks ────────────────────────────────
#[test]
fn a_user_tool_result_becomes_a_tool_message_carrying_the_call_id() {
// Ollama correlates a tool result to its call by tool_call_id. Lose
// it and the model gets an answer it cannot attach to a question.
let msg = json!({
"role": "user",
"content": [{
"type": "tool_result",
"tool_use_id": "call_abc",
"content": "42"
}]
});
let out = translate_message(&msg);
assert_eq!(
out,
vec![json!({
"role": "tool",
"content": "42",
"tool_call_id": "call_abc"
})]
);
}
#[test]
fn a_structured_tool_result_is_stringified_as_json_not_debug_formatted() {
let msg = json!({
"role": "user",
"content": [{
"type": "tool_result",
"tool_use_id": "c1",
"content": {"rows": 3}
}]
});
let out = translate_message(&msg);
assert_eq!(
out[0]["content"], "{\"rows\":3}",
"a structured result must reach the model as JSON it can read"
);
}
#[test]
fn a_user_text_block_becomes_a_user_message() {
let msg = json!({
"role": "user",
"content": [{"type": "text", "text": "hello"}]
});
assert_eq!(
translate_message(&msg),
vec![json!({"role": "user", "content": "hello"})]
);
}
#[test]
fn mixed_user_blocks_split_into_one_message_each_preserving_order() {
let msg = json!({
"role": "user",
"content": [
{"type": "tool_result", "tool_use_id": "c1", "content": "ok"},
{"type": "text", "text": "now what?"}
]
});
let out = translate_message(&msg);
assert_eq!(out.len(), 2, "each block is its own message: {out:?}");
assert_eq!(out[0]["role"], "tool");
assert_eq!(out[1], json!({"role": "user", "content": "now what?"}));
}
#[test]
fn assistant_text_and_tool_use_collapse_into_a_single_message() {
let msg = json!({
"role": "assistant",
"content": [
{"type": "text", "text": "Let me look."},
{"type": "tool_use", "id": "call_1", "name": "search", "input": {"q": "anvil"}}
]
});
let out = translate_message(&msg);
assert_eq!(
out.len(),
1,
"assistant turns are one message, got: {out:?}"
);
assert_eq!(out[0]["content"], "Let me look.");
assert_eq!(
out[0]["tool_calls"],
json!([{
"id": "call_1",
"type": "function",
"function": {"name": "search", "arguments": {"q": "anvil"}}
}])
);
}
#[test]
fn an_assistant_message_without_tool_use_omits_the_tool_calls_key() {
// Sending `tool_calls: []` is not the same as sending nothing —
// some models take the empty list as "you already tried tools".
let msg = json!({
"role": "assistant",
"content": [{"type": "text", "text": "done"}]
});
let out = translate_message(&msg);
assert!(
out[0].get("tool_calls").is_none(),
"empty tool_calls must be omitted entirely, got: {}",
out[0]
);
}
#[test]
fn a_tool_use_missing_its_input_sends_an_empty_object_not_null() {
// Ollama rejects a null `arguments`; an empty object is a valid
// zero-argument call.
let msg = json!({
"role": "assistant",
"content": [{"type": "tool_use", "id": "c", "name": "now"}]
});
let out = translate_message(&msg);
assert_eq!(out[0]["tool_calls"][0]["function"]["arguments"], json!({}));
}
#[test]
fn block_type_of_an_untyped_block_is_empty_not_a_panic() {
assert_eq!(block_type(&json!({})), "");
assert_eq!(block_type(&json!({"type": 7})), "");
assert_eq!(block_type(&json!({"type": "text"})), "text");
}
#[test]
fn stringify_distinguishes_a_json_string_from_a_missing_value() {
assert_eq!(stringify(Some(&json!("raw"))), "raw");
assert_eq!(stringify(Some(&json!(12))), "12");
assert_eq!(stringify(None), "");
}
// ── Anvil → Ollama: tools and options ─────────────────────────────
#[test]
fn anvil_tool_defs_become_ollama_function_tools() {
let req = json!({
"tools": [{
"name": "get_weather",
"description": "Look up weather",
"input_schema": {"type": "object", "properties": {"city": {"type": "string"}}}
}]
});
let tools = ollama_tools(&req);
assert_eq!(
tools,
vec![json!({
"type": "function",
"function": {
"name": "get_weather",
"description": "Look up weather",
"parameters": {"type": "object", "properties": {"city": {"type": "string"}}}
}
})],
"input_schema must land under function.parameters"
);
}
#[test]
fn a_request_without_tools_yields_an_empty_list_so_the_key_is_omitted() {
// call_ollama only inserts `tools` when this is non-empty.
assert!(ollama_tools(&json!({})).is_empty());
assert!(ollama_tools(&json!({"tools": "not-an-array"})).is_empty());
}
#[test]
fn a_tool_without_a_schema_gets_an_empty_object_not_null() {
let tools = ollama_tools(&json!({"tools": [{"name": "ping"}]}));
assert_eq!(tools[0]["function"]["parameters"], json!({}));
assert_eq!(tools[0]["function"]["description"], "");
}
#[test]
fn temperature_and_max_tokens_map_to_ollama_option_names() {
let opts = ollama_options(&json!({"temperature": 0.2, "max_tokens": 512}))
.expect("both options are set");
assert_eq!(opts["temperature"], json!(0.2));
assert_eq!(
opts["num_predict"],
json!(512),
"Ollama spells max_tokens `num_predict`"
);
}
#[test]
fn explicitly_null_options_are_dropped_rather_than_forwarded() {
// Anvil sends the key with a null when the caller left it unset;
// forwarding `temperature: null` makes Ollama 400 the request.
assert!(
ollama_options(&json!({"temperature": null, "max_tokens": null})).is_none(),
"all-null options must produce no options block at all"
);
}
#[test]
fn one_set_option_still_produces_an_options_block() {
let opts = ollama_options(&json!({"temperature": null, "max_tokens": 64}))
.expect("max_tokens alone is enough");
assert_eq!(opts["num_predict"], json!(64));
assert!(opts.get("temperature").is_none());
}
#[test]
fn unrecognised_options_produce_no_options_block() {
assert!(ollama_options(&json!({"top_k": 40})).is_none());
}
// ── Ollama → Anvil: responses ─────────────────────────────────────
#[test]
fn a_plain_completion_maps_content_and_token_usage() {
let raw = json!({
"message": {"role": "assistant", "content": "Hello!"},
"done_reason": "stop",
"prompt_eval_count": 12,
"eval_count": 5
});
let out = parse_ollama_response(&raw);
assert_eq!(out["content"], "Hello!");
assert_eq!(out["stop_reason"], "end_turn");
assert_eq!(out["tool_uses"], json!([]));
assert_eq!(
out["usage"],
json!({"input_tokens": 12, "output_tokens": 5})
);
}
#[test]
fn missing_token_counters_default_to_zero_rather_than_null() {
// Anvil's usage accounting sums these; a null would poison the total.
let out = parse_ollama_response(&json!({"message": {"content": "hi"}}));
assert_eq!(out["usage"]["input_tokens"], json!(0));
assert_eq!(out["usage"]["output_tokens"], json!(0));
}
#[test]
fn a_response_with_no_message_yields_empty_content_not_a_panic() {
let out = parse_ollama_response(&json!({}));
assert_eq!(out["content"], "");
assert_eq!(out["tool_uses"], json!([]));
assert_eq!(out["stop_reason"], "end_turn");
}
#[test]
fn tool_calls_are_mapped_to_anvil_tool_uses() {
let raw = json!({
"message": {
"content": "",
"tool_calls": [{
"id": "call_9",
"function": {"name": "search", "arguments": {"q": "rust"}}
}]
}
});
let out = parse_ollama_response(&raw);
assert_eq!(
out["tool_uses"],
json!([{"id": "call_9", "name": "search", "input": {"q": "rust"}}])
);
}
#[test]
fn a_tool_call_wins_over_done_reason_when_setting_stop_reason() {
// Ollama reports done_reason "stop" even on a tool call. Passing
// that through as end_turn ends the agent loop before the tool runs.
let raw = json!({
"done_reason": "stop",
"message": {
"content": "",
"tool_calls": [{"function": {"name": "f", "arguments": {}}}]
}
});
assert_eq!(
parse_ollama_response(&raw)["stop_reason"],
"tool_use",
"a response carrying tool calls must not be reported as end_turn"
);
}
#[test]
fn a_tool_call_without_an_id_gets_a_stable_positional_id() {
// Anvil pairs the eventual tool_result to the call by id, so an
// absent id has to become something — and two calls in one response
// must not collide.
let raw = json!({
"message": {"content": "", "tool_calls": [
{"function": {"name": "a", "arguments": {}}},
{"function": {"name": "b", "arguments": {}}}
]}
});
let uses = parse_ollama_response(&raw)["tool_uses"].clone();
assert_eq!(uses[0]["id"], "ollama-0");
assert_eq!(uses[1]["id"], "ollama-1");
}
#[test]
fn a_tool_call_with_no_function_object_still_produces_a_use() {
let raw = json!({"message": {"content": "", "tool_calls": [{"id": "x"}]}});
let uses = parse_ollama_response(&raw)["tool_uses"].clone();
assert_eq!(uses[0], json!({"id": "x", "name": "", "input": {}}));
}
#[test]
fn done_reason_length_maps_to_max_tokens() {
assert_eq!(stop_reason(Some("length"), false), "max_tokens");
}
#[test]
fn an_unknown_or_absent_done_reason_is_end_turn() {
assert_eq!(stop_reason(Some("stop"), false), "end_turn");
assert_eq!(stop_reason(None, false), "end_turn");
assert_eq!(stop_reason(Some("something_new"), false), "end_turn");
}
#[test]
fn tool_use_overrides_even_a_length_stop() {
assert_eq!(stop_reason(Some("length"), true), "tool_use");
}
#[test]
fn object_arguments_pass_through_unchanged() {
assert_eq!(normalize_arguments(Some(&json!({"a": 1}))), json!({"a": 1}));
}
#[test]
fn string_encoded_arguments_are_parsed_into_an_object() {
// Several small models emit `arguments` as a JSON *string*.
assert_eq!(
normalize_arguments(Some(&json!("{\"city\":\"Oslo\"}"))),
json!({"city": "Oslo"}),
"a JSON-string argument blob must be decoded, not passed as text"
);
}
#[test]
fn unparseable_string_arguments_are_preserved_under_raw() {
// Losing them entirely would make the failure impossible to debug.
assert_eq!(
normalize_arguments(Some(&json!("not json at all"))),
json!({"_raw": "not json at all"})
);
}
#[test]
fn absent_arguments_become_an_empty_object() {
assert_eq!(normalize_arguments(None), json!({}));
assert_eq!(normalize_arguments(Some(&json!(null))), json!({}));
}
// ── Dispatch ──────────────────────────────────────────────────────
#[tokio::test]
async fn a_request_for_another_provider_is_refused_by_name() {
let err = run_one(
&test_config("http://127.0.0.1:1"),
&json!({"provider": "anthropic"}),
)
.await
.unwrap_err();
assert!(
err.contains("anthropic"),
"the refusal must name the provider it cannot serve, got: {err}"
);
}
#[tokio::test]
async fn a_request_with_no_provider_is_refused() {
let err = run_one(&test_config("http://127.0.0.1:1"), &json!({}))
.await
.unwrap_err();
assert!(err.contains("provider"), "got: {err}");
}
fn test_config(ollama_url: &str) -> RunnerConfig {
RunnerConfig::for_test("http://server.invalid", ollama_url)
}
}
src/runner/loop_runner.rs +44 −4
@@ -354,9 +354,29 @@
workspace::is_disposable(config.ephemeral),
);
// A job with no repo checkout still needs a real, job-scoped directory.
// The old `Path::new(".")` fallback meant the runner daemon's cwd, which
// artifact confinement then treats as the publishable tree — see
// `workspace::bare`.
let bare_ws = match workspace_path {
Some(_) => None,
None => match workspace::bare(&config.work_dir_path(), slot, &config.runner_id) {
Ok(dir) => Some(dir),
Err(e) => {
log_reporter
.append(&format!("Workspace preparation failed: {e}"))
.await;
flush_handle.abort();
log_reporter.drain().await;
return Err(e);
}
},
};
let ws = workspace_path
.as_deref()
.or(bare_ws.as_deref())
.unwrap_or_else(|| std::path::Path::new("."));
.expect("a checkout workspace or a bare one is always set");
// Start services
let services = job.get("services");
@@ -384,7 +404,21 @@
let mut env = HashMap::new();
if let Some(job_env) = job.get("env").and_then(|v| v.as_object()) {
for (k, v) in job_env {
// An unquoted YAML scalar (`RETRIES: 3`) is a number here; reading
env.insert(k.clone(), v.as_str().unwrap_or("").to_string());
// only strings handed the step an empty value that looked set.
match crate::runner::env_value(v) {
Some(val) => {
env.insert(k.clone(), val);
}
None => {
log_reporter
.append(&format!(
" Ignoring env '{k}': a list or map has no value a \
process can be given"
))
.await;
}
}
}
}
if let Some(ref ctx) = svc_context {
@@ -457,8 +491,14 @@
artifact_specs.len()
))
.await;
let count = artifacts::upload_artifacts(config, job_id, ws, &artifact_specs).await;
let outcome = artifacts::upload_artifacts(config, job_id, ws, &artifact_specs).await;
// Refusals and no-matches belong in the job log: whoever's pipeline
// collected nothing cannot read the runner host's stderr, and
// "Uploaded 0 artifact(s)" on its own gives them nothing to act on.
for notice in &outcome.notices {
log_reporter.append(&format!(" {notice}")).await;
}
log_reporter
.append(&format!("Uploaded {} artifact(s)", outcome.uploaded))
.append(&format!("Uploaded {count} artifact(s)"))
.await;
}
src/runner/mod.rs +52 −0
@@ -19,3 +19,55 @@
pub use config::RunnerConfig;
/// Render a JSON value from `.anvil.yml` as an environment-variable value.
///
/// `.anvil.yml` is YAML, so `POSTGRES_PASSWORD: 12345` and
/// `MYSQL_ALLOW_EMPTY_PASSWORD: 1` — the natural, unquoted way to write them —
/// arrive here as JSON *numbers*, and `PGSSLMODE: false` as a bool. Reading
/// these with `as_str().unwrap_or("")` turned every one of them into an empty
/// value: the container came up with a blank password, its health probe timed
/// out, and the job failed with "did not become healthy" while the pipeline
/// file looked perfectly correct.
///
/// Scalars render as their plain text. An explicit `null` becomes an empty
/// value, which is what YAML's bare `KEY:` means. A list or map has no sensible
/// rendering, so it returns `None` and the caller reports it rather than
/// quietly substituting something that looks like a real setting.
pub fn env_value(v: &serde_json::Value) -> Option<String> {
use serde_json::Value;
match v {
Value::String(s) => Some(s.clone()),
Value::Number(n) => Some(n.to_string()),
Value::Bool(b) => Some(b.to_string()),
Value::Null => Some(String::new()),
Value::Array(_) | Value::Object(_) => None,
}
}
#[cfg(test)]
mod env_value_tests {
use super::env_value;
use serde_json::json;
#[test]
fn a_yaml_scalar_keeps_the_text_the_author_wrote() {
assert_eq!(env_value(&json!("secret")).as_deref(), Some("secret"));
assert_eq!(env_value(&json!(12345)).as_deref(), Some("12345"));
assert_eq!(env_value(&json!(1)).as_deref(), Some("1"));
assert_eq!(env_value(&json!(false)).as_deref(), Some("false"));
assert_eq!(env_value(&json!(1.5)).as_deref(), Some("1.5"));
}
#[test]
fn an_explicit_null_is_an_empty_value_not_a_rejection() {
// Bare `KEY:` in YAML is a deliberate empty value.
assert_eq!(env_value(&json!(null)).as_deref(), Some(""));
}
#[test]
fn a_list_or_map_has_no_rendering_and_is_reported_instead() {
assert!(env_value(&json!(["a"])).is_none());
assert!(env_value(&json!({"a": 1})).is_none());
}
}
src/runner/prepare.rs +12 −2
@@ -602,8 +602,18 @@
// and reproducible while the daemon is unchanged. It is deliberately not
// cached, so this re-probes.
let (os, arch) = docker_platform();
assert!(!os.is_empty() && !arch.is_empty());
assert_eq!(docker_platform(), (os, arch));
// Split, so a failure names which half is missing instead of just
// reporting "assertion failed".
assert!(!os.is_empty(), "docker_platform returned an empty os");
assert!(
!arch.is_empty(),
"docker_platform returned an empty arch (os={os})"
);
assert_eq!(
docker_platform(),
(os.clone(), arch.clone()),
"re-probing changed the platform from {os}/{arch}; the prepared-image tag would churn"
);
}
#[test]
src/runner/service_manager.rs +251 −7
@@ -159,8 +159,11 @@
.get("ports")
.and_then(|v| v.as_array())
.and_then(|a| a.first())
// Same YAML-scalar problem as above: an unquoted
// `ports: [6379]` is a number, and skipping it silently
// fell through to the well-known-port guess.
.and_then(crate::runner::env_value)
.and_then(|s| s.split(':').next().map(str::to_string))
.and_then(|p| p.as_str())
.and_then(|s| s.split(':').next())
.and_then(|s| s.parse::<u16>().ok())
})
.or_else(|| well_known_port(image));
@@ -234,7 +237,8 @@
if let Some(env_map) = svc_config.get("env").and_then(|v| v.as_object()) {
for (k, v) in env_map {
let val = crate::runner::env_value(v)
.ok_or_else(|| format!("env value for '{k}' must be a scalar, got {v}"))?;
let val = v.as_str().unwrap_or("");
args.push("-e".into());
args.push(format!("{k}={val}"));
}
@@ -242,9 +246,11 @@
if let Some(ports) = svc_config.get("ports").and_then(|v| v.as_array()) {
for port in ports {
if let Some(p) = port.as_str() {
// `ports: [6379]` is unquoted YAML and arrives as a number; taking
// only strings silently published nothing.
let p = crate::runner::env_value(port)
.ok_or_else(|| format!("port entry must be a scalar, got {port}"))?;
args.push("-p".into());
args.push(p);
args.push("-p".into());
args.push(p.to_string());
}
}
}
@@ -342,6 +348,244 @@
let args = build_run_args("alias-name", &svc, "c", "net").unwrap();
let alias_pos = args.iter().position(|a| a == "--network-alias").unwrap();
assert_eq!(args.get(alias_pos + 1), Some(&"alias-name".to_string()));
}
#[test]
fn build_run_args_rejects_a_service_with_no_image() {
// The caller prefixes this with the service name, so it must name the
// missing field and nothing else.
let err = build_run_args("cache", &serde_json::json!({}), "c", "n").unwrap_err();
assert!(err.contains("image"), "got: {err}");
}
#[test]
fn build_run_args_detaches_and_names_the_container_on_the_job_network() {
// A service started in the foreground would block the job forever;
// one off the job network is unreachable from it.
let svc = serde_json::json!({ "image": "redis:7" });
let args = build_run_args("redis", &svc, "anvil-ci-svc-J-redis", "anvil-ci-J").unwrap();
assert_eq!(args[0], "run");
assert!(args.contains(&"--detach".to_string()), "args: {args:?}");
let name_pos = args.iter().position(|a| a == "--name").unwrap();
assert_eq!(args.get(name_pos + 1).unwrap(), "anvil-ci-svc-J-redis");
let net_pos = args.iter().position(|a| a == "--network").unwrap();
assert_eq!(args.get(net_pos + 1).unwrap(), "anvil-ci-J");
}
#[test]
fn build_run_args_forwards_env_as_a_separate_flag_and_value() {
// Collapsing these into a single "-e K=V" argument would have docker
// read the whole thing as one flag and fail.
let svc = serde_json::json!({
"image": "postgres:16",
"env": { "POSTGRES_PASSWORD": "secret" }
});
let args = build_run_args("db", &svc, "c", "n").unwrap();
let e_pos = args.iter().position(|a| a == "-e").unwrap();
assert_eq!(args.get(e_pos + 1).unwrap(), "POSTGRES_PASSWORD=secret");
}
#[test]
fn build_run_args_renders_an_unquoted_yaml_number_as_its_text() {
// Regression: `as_str().unwrap_or("")` turned `POSTGRES_PASSWORD: 12345`
// into `POSTGRES_PASSWORD=`, so the container booted with a blank
// password, the health probe timed out, and the job failed with an
// opaque "did not become healthy" while the pipeline file looked right.
let svc = serde_json::json!({
"image": "postgres:16",
"env": { "POSTGRES_PASSWORD": 12345 }
});
let args = build_run_args("db", &svc, "c", "n").unwrap();
let e_pos = args.iter().position(|a| a == "-e").unwrap();
assert_eq!(args.get(e_pos + 1).unwrap(), "POSTGRES_PASSWORD=12345");
}
#[test]
fn build_run_args_renders_an_unquoted_yaml_bool_as_its_text() {
let svc = serde_json::json!({ "image": "x", "env": { "TLS": false } });
let args = build_run_args("s", &svc, "c", "n").unwrap();
let e_pos = args.iter().position(|a| a == "-e").unwrap();
assert_eq!(args.get(e_pos + 1).unwrap(), "TLS=false");
}
#[test]
fn build_run_args_rejects_a_structured_env_value_instead_of_blanking_it() {
// An empty value looks like a real setting; a named error does not.
let svc = serde_json::json!({ "image": "x", "env": { "NESTED": {"a": 1} } });
let err = build_run_args("s", &svc, "c", "n").unwrap_err();
assert!(
err.contains("NESTED"),
"the message must name the key: {err}"
);
assert!(err.contains("scalar"), "the message must say why: {err}");
}
#[test]
fn build_run_args_publishes_every_declared_port_before_the_image() {
let svc = serde_json::json!({
"image": "redis:7",
"ports": ["6379:6379", "16379:16379"]
});
let args = build_run_args("redis", &svc, "c", "n").unwrap();
let published: Vec<&String> = args
.iter()
.enumerate()
.filter(|(i, _)| *i > 0 && args[i - 1] == "-p")
.map(|(_, a)| a)
.collect();
assert_eq!(published, vec!["6379:6379", "16379:16379"]);
let img_pos = args.iter().position(|a| a == "redis:7").unwrap();
let last_p = args.iter().rposition(|a| a == "-p").unwrap();
assert!(
last_p < img_pos,
"docker flags must precede the image, got: {args:?}"
);
}
#[test]
fn build_run_args_publishes_an_unquoted_numeric_port() {
// `ports: [6379]` is how anyone would write it in YAML; dropping it
// meant the port was silently never published.
let svc = serde_json::json!({ "image": "redis:7", "ports": [6379] });
let args = build_run_args("redis", &svc, "c", "n").unwrap();
let p_pos = args
.iter()
.position(|a| a == "-p")
.expect("a numeric port must still be published");
assert_eq!(args.get(p_pos + 1).unwrap(), "6379");
}
#[test]
fn build_run_args_drops_non_string_entries_from_a_list_command() {
let svc = serde_json::json!({
"image": "busybox",
"command": ["sh", 7, "-c", null, "echo hi"]
});
let args = build_run_args("svc", &svc, "c", "n").unwrap();
let img_pos = args.iter().position(|a| a == "busybox").unwrap();
assert_eq!(&args[img_pos + 1..], &["sh", "-c", "echo hi"]);
}
#[test]
fn build_run_args_collapses_runs_of_whitespace_in_a_string_command() {
let svc = serde_json::json!({
"image": "minio/minio:latest",
"command": "server /data\t--console"
});
// Service name deliberately differs from the image: it is also
// present in argv as the --network-alias value.
let args = build_run_args("objstore", &svc, "c", "n").unwrap();
let img_pos = args.iter().position(|a| a == "minio/minio:latest").unwrap();
assert_eq!(&args[img_pos + 1..], &["server", "/data", "--console"]);
}
#[test]
fn build_run_args_ignores_a_command_that_is_neither_string_nor_list() {
let svc = serde_json::json!({ "image": "busybox", "command": {"run": "x"} });
let args = build_run_args("svc", &svc, "c", "n").unwrap();
assert_eq!(
args.last().unwrap(),
"busybox",
"an unusable command must leave the image as the last arg"
);
}
#[test]
fn well_known_ports_cover_the_images_anvil_ships_defaults_for() {
// This is the fallback used when a service declares no port, so a
// wrong number silently points the job at nothing.
assert_eq!(well_known_port("postgres:16"), Some(5432));
assert_eq!(well_known_port("mysql:8"), Some(3306));
assert_eq!(well_known_port("mariadb:11"), Some(3306));
assert_eq!(well_known_port("redis:7"), Some(6379));
assert_eq!(well_known_port("memcached:1.6"), Some(11211));
assert_eq!(well_known_port("mongo:7"), Some(27017));
assert_eq!(
well_known_port("docker.elastic.co/elasticsearch/elasticsearch:8.13.0"),
Some(9200)
);
assert_eq!(well_known_port("rabbitmq:3-management"), Some(5672));
assert_eq!(well_known_port("minio/minio:latest"), Some(9000));
}
#[test]
fn well_known_port_matches_a_registry_qualified_image_name() {
// Images are routinely pulled through a mirror or a private registry.
assert_eq!(
well_known_port("public.ecr.aws/docker/library/redis:7"),
Some(6379)
);
assert_eq!(well_known_port("ghcr.io/acme/postgres-ha:16"), Some(5432));
}
#[test]
fn well_known_port_is_case_insensitive() {
assert_eq!(well_known_port("POSTGRES:16"), Some(5432));
}
#[test]
fn an_unrecognised_image_has_no_well_known_port() {
// The caller must then fall back to whatever the service declared,
// rather than exporting a confidently wrong SVC_PORT.
assert_eq!(well_known_port("acme/custom-thing:1"), None);
assert_eq!(well_known_port(""), None);
}
#[test]
fn health_checks_exec_inside_the_named_container() {
// Running the probe on the *host* would test the wrong machine — and
// usually find no client binary there at all.
for image in ["postgres:16", "redis:7", "mysql:8", "mongo:7"] {
let cmd = health_check_cmd(image, "anvil-ci-svc-J-x")
.unwrap_or_else(|| panic!("{image} should have a health check"));
assert_eq!(&cmd[0..2], &["docker", "exec"], "{image}: {cmd:?}");
assert_eq!(cmd[2], "anvil-ci-svc-J-x", "{image}: {cmd:?}");
}
}
#[test]
fn each_service_is_probed_with_its_own_clients_readiness_command() {
let probe = |img| health_check_cmd(img, "c").unwrap();
assert!(probe("redis:7").contains(&"redis-cli".to_string()));
assert!(probe("mysql:8").contains(&"mysqladmin".to_string()));
assert!(probe("mariadb:11").contains(&"mysqladmin".to_string()));
assert!(probe("mongo:7").contains(&"mongosh".to_string()));
}
#[test]
fn mysql_readiness_is_probed_silently_so_the_log_is_not_flooded() {
// The probe runs twice a second for up to a minute.
let cmd = health_check_cmd("mysql:8", "c").unwrap();
assert!(cmd.contains(&"--silent".to_string()), "got: {cmd:?}");
}
#[test]
fn an_unknown_image_has_no_health_check_so_the_caller_falls_back_to_waiting() {
assert!(health_check_cmd("acme/custom:1", "c").is_none());
}
#[test]
src/runner/service_mode.rs +51 −0
@@ -219,6 +219,57 @@
assert!(validate_instance_name("A").is_ok());
}
// The instance name reaches a systemd unit filename and a launchd label,
// so the rejection message is the only place a user learns what is legal.
// Asserting `is_err()` alone would keep passing if every distinct rule
// collapsed into one unhelpful "invalid name".
#[test]
fn an_empty_instance_name_is_diagnosed_as_empty() {
let err = validate_instance_name("").unwrap_err();
assert!(
err.contains("empty"),
"an empty name should be diagnosed as empty, got: {err}"
);
}
#[test]
fn a_leading_dash_is_diagnosed_as_a_bad_first_character() {
// Distinct from the invalid-character rule: `-foo` is rejected for
// where the dash sits, not for containing one at all.
let err = validate_instance_name("-foo").unwrap_err();
assert!(
err.contains("start with"),
"the message must point at the first character, got: {err}"
);
assert!(
validate_instance_name("foo-bar").is_ok(),
"a dash is legal once it is not leading"
);
}
#[test]
fn an_invalid_character_is_named_along_with_the_allowed_set() {
for (name, offender) in [("foo bar", ' '), ("foo/bar", '/'), ("foo.bar", '.')] {
let err = validate_instance_name(name).unwrap_err();
assert!(
err.contains(name),
"the message must quote the rejected name, got: {err}"
);
assert!(
err.contains(&format!("{offender:?}")),
"the message must name the offending character {offender:?}, got: {err}"
);
assert!(
err.contains("letters, digits"),
"the message must state what IS allowed, got: {err}"
);
}
}
#[test]
fn validate_instance_name_rejects_invalid() {
assert!(validate_instance_name("").is_err());
src/runner/workspace.rs +101 −0
@@ -300,6 +300,33 @@
}
}
/// Working directory for a job that has no repo checkout (no `repo_url` /
/// `commit_sha` in the claim).
///
/// Such a job used to run in `Path::new(".")` — the runner *daemon's* current
/// directory. That was already untidy (a bare step wrote into whatever
/// directory the operator happened to launch from), but it became a security
/// boundary once artifact collection started confining paths to "the
/// workspace": a runner started by hand from `$HOME` would treat the
/// operator's home directory as the tree it is willing to publish, so an
/// artifact spec of `.ssh/id_ed25519` resolved *inside* the boundary and
/// passed the check. A real directory under `work_dir` keeps that boundary
/// meaningful.
///
/// Named through the same `workspace_path` scheme as a checkout so the pruning
/// sweep recognises and ages it out like any other workspace, and scoped per
/// runner instance and slot so two concurrent bare jobs never share a tree.
pub fn bare(
work_dir: &Path,
slot: u32,
runner_id: &str,
) -> Result<PathBuf, Box<dyn std::error::Error + Send + Sync>> {
let dir = workspace_path(work_dir, "_bare", "_bare", runner_id, slot);
std::fs::create_dir_all(&dir)?;
write_lock(&dir);
Ok(dir)
}
/// Prepare workspace for a job: clone or fetch, then checkout the target SHA.
/// Returns the workspace directory path.
pub fn prepare(
@@ -442,6 +469,80 @@
const WORK: &str = "/work";
const RUNNER_A: &str = "ad2a1b80-5180-43ed-81be-3ae49e6ccdf9";
const RUNNER_B: &str = "ff60fbf0-5c9e-4c01-b853-582a6c3b1ab2";
// ── bare (no-checkout) workspaces ─────────────────────────────────
#[test]
fn a_bare_job_gets_a_real_directory_under_the_work_dir() {
// Regression: a job with no repo checkout ran in `Path::new(".")` —
// the runner daemon's cwd. Artifact collection confines paths to "the
// workspace", so on a runner started by hand from $HOME that made the
// operator's home directory the publishable tree, and a spec of
// `.ssh/id_ed25519` resolved *inside* the boundary.
let root = crate::testutil::TempDir::new("ws-bare");
let dir = bare(root.path(), 0, RUNNER_A).unwrap();
assert!(dir.is_dir(), "bare must create the directory: {dir:?}");
assert!(
dir.starts_with(root.path()),
"a bare workspace must live under work_dir, got {dir:?}"
);
assert_ne!(
dir.canonicalize().unwrap(),
std::env::current_dir().unwrap().canonicalize().unwrap(),
"a bare workspace must never be the daemon's own cwd"
);
}
#[test]
fn two_slots_running_bare_jobs_do_not_share_a_tree() {
// Concurrent bare jobs writing into one directory would let each
// other's leftovers be collected as artifacts.
let root = crate::testutil::TempDir::new("ws-bare-slots");
let a = bare(root.path(), 0, RUNNER_A).unwrap();
let b = bare(root.path(), 1, RUNNER_A).unwrap();
assert_ne!(a, b);
}
#[test]
fn two_runners_running_bare_jobs_do_not_share_a_tree() {
let root = crate::testutil::TempDir::new("ws-bare-runners");
let a = bare(root.path(), 0, RUNNER_A).unwrap();
let b = bare(root.path(), 0, RUNNER_B).unwrap();
assert_ne!(a, b);
}
#[test]
fn a_bare_workspace_is_named_so_the_pruning_sweep_recognises_it() {
// Otherwise these accumulate forever on a stateful runner.
let root = crate::testutil::TempDir::new("ws-bare-sweep");
let dir = bare(root.path(), 2, RUNNER_A).unwrap();
assert!(
is_workspace_name(&dir),
"the sweep must recognise {dir:?} as a workspace"
);
}
#[test]
fn preparing_a_bare_workspace_twice_reuses_it() {
// These runners are stateful by design; a bare job's warm directory
// should survive between runs like a checkout does.
let root = crate::testutil::TempDir::new("ws-bare-reuse");
let first = bare(root.path(), 0, RUNNER_A).unwrap();
std::fs::write(first.join("cache.bin"), b"warm").unwrap();
let second = bare(root.path(), 0, RUNNER_A).unwrap();
assert_eq!(first, second);
assert!(second.join("cache.bin").exists(), "reuse must not wipe");
}
#[test]
fn two_runners_sharing_a_work_dir_get_separate_workspaces() {
src/testutil.rs +76 −0
@@ -1,0 +1,76 @@
//! Shared helpers for unit tests.
//!
//! Compiled only under `cfg(test)`, so nothing here ships in the binary.
use std::path::{Path, PathBuf};
/// A scratch directory that removes itself when it goes out of scope —
/// including when the scope is left by a panicking assertion.
///
/// Cleaning up with a `remove_dir_all` at the end of a test only works when the
/// test passes, which is exactly backwards: the runs that leave debris are the
/// failing ones. Anvil's own runners are stateful and long-lived, so `/tmp` is
/// not reset between jobs and a repeatedly failing test accumulates directories
/// (some holding the 100MB fixture the artifact size-cap test writes) until a
/// later job on that worker dies of ENOSPC.
///
/// The name is derived from `tag` and the pid rather than a timestamp, so it is
/// *reclaimable*: a directory orphaned by a hard kill — which no `Drop` can
/// defend against — is cleared by the next run of the same test instead of
/// living forever. `tag` must therefore be unique per test within a file, since
/// tests run in parallel.
pub struct TempDir {
path: PathBuf,
}
impl TempDir {
pub fn new(tag: &str) -> Self {
let path = std::env::temp_dir().join(format!("anvil-t-{tag}-{}", std::process::id()));
// Clear anything a previously killed run left behind, so each test
// starts from a known-empty tree.
let _ = std::fs::remove_dir_all(&path);
std::fs::create_dir_all(&path).expect("create scratch directory");
Self { path }
}
pub fn path(&self) -> &Path {
&self.path
}
/// The scratch directory with symlinks resolved — what confinement checks
/// and `strip_prefix` comparisons need, since `/tmp` is itself a symlink on
/// some platforms.
pub fn canonical(&self) -> PathBuf {
self.path.canonicalize().expect("canonicalize scratch dir")
}
pub fn join(&self, rel: &str) -> PathBuf {
self.path.join(rel)
}
/// Write `contents` to `rel`, creating parent directories. Returns the path.
pub fn write(&self, rel: &str, contents: &str) -> PathBuf {
let p = self.path.join(rel);
if let Some(parent) = p.parent() {
std::fs::create_dir_all(parent).expect("create parent directory");
}
std::fs::write(&p, contents).expect("write fixture file");
p
}
}
impl Drop for TempDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.path);
}
}
/// True when the process is running as root, where discretionary permission
/// checks are bypassed. Tests whose whole point is "this is refused" have to
/// skip rather than silently pass for the wrong reason — CI runs as root in a
/// container.
#[cfg(unix)]
pub fn running_as_root() -> bool {
// Safe: getuid has no preconditions and cannot fail.
(unsafe { libc::getuid() }) == 0
}