Conversation
) Found by a security-control mutation campaign: **the cap was a dead control.** ## What the campaign did Ten security controls, each deleted or weakened to see whether anything went red. Nine were alive — the zip-slip guard, the extracted-mode sanitiser, the systemd control-character rejection, the quadlet filter, the error-message escaping, the signature verification (three ways), the project-name sanitiser. **The 16 MiB read cap was not.** Raising `MAX_FILE_BYTES` to a terabyte left the whole lib and bins suite green. ## Why it was dead `read_capped_from` implements the cap and *is* unit-tested — but with an explicit limit passed in. So the mechanism was proven and **the call sites' choice of limit was not**. Neither `-f <file>` nor `-f -` had anything asserting they pass `MAX_FILE_BYTES` rather than something larger. Neither can be closed at unit level: one reads the real stdin, and the constant's effect is only observable through the binary. So these are CLI tests that feed a document either side of the cap — a file for `-f`, a pipe for `-f -` — and assert the refusal. ## Verified by watching it fail With the constant raised to a terabyte, exactly the two refusal tests go red and the two acceptance tests stay green. Restored, all four pass. ## Two things I got wrong first, both worth recording **My first mutation of this control looked like a kill and was not.** Replacing the constant with `u64::MAX` makes `take(max + 1)` overflow and panic, so every test fails for the wrong reason — and I nearly recorded the control as alive on that basis. The commit message carries the warning for whoever mutates it next. **Three other controls looked dead and were not.** `sanitize_project_name` and the zip-slip guard survived until I noticed the driver only ran `cargo test --lib`, and `resolve` is a module of `main.rs`. Its four tests exist and run — under `--bins`. The driver now runs both, which is the same trap the product context already records for `startup`. ## Test plan - 4 new CLI tests, both sides of the threshold on both paths. - The clean mutation (1 TiB, no overflow) kills exactly the two refusal tests. - `cargo fmt --all --check`, `cargo clippy --locked --all-targets --all-features -- -D warnings`, and every test target green: lib 1524, bins 81, and all integration targets. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Part of #1326. ## The numbers | what | lines | |---|---| | `cargo llvm-cov --lib --bins` — what the CI job can see | **79.39%** | | `cargo llvm-cov --all-features` — the full suite | **91.52%** | | the gate | **75** → **79** | **podup is not short of the org standard's 90%.** It clears it. The coverage job just cannot see that: it runs on a plain runner, every integration test begins `if podman().await.is_none() { return; }`, and with them skip the coverage of `dispatch.rs`, `dispatch/rest.rs` and `autostart_cmd.rs` — all reading 0.00% under `--lib --bins` and all exercised heavily by the suite that cannot run there. ## What this PR does and does not do It closes four points of **silent slack**: coverage could fall from 79 to 75 with nothing saying so. Ratcheting to what holds today is what the testing standard asks when adopting a floor — *"locks in a state that holds rather than demanding new work"*. It does **not** claim to enforce 90. Nothing does, and that is #1326: the lane that can run the integration tests measures no coverage at all, so the standard's number is satisfied by the code and checked nowhere. The threshold carries a comment saying which number is which, so the next person does not read 79 as a failure to reach 90 and go looking for tests that already exist. ## Test plan - Both figures measured with `cargo llvm-cov` on this tree today, not inferred. - YAML validated. - No source changed. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Part of #1322. ## What it was Three tests published `127.0.0.1:18081` and a fourth `18080`. Any two running at once lost the bind: ``` pasta failed with exit code 1: Listen failed for HOST TCP port 127.0.0.1/18080: Address already in use ``` At eight test threads that is close to certain rather than unlucky. It was **one of the three failures that survived even at `--test-threads=1`** on the Podman 6 VM, which is how it surfaced: everything else at that level was #1207, and this one was ours. ## The fix `free_port` — written this morning for the registry test — moves up to the suite root, and all four fixtures take a port chosen at run time. There is a window between releasing the port and the container binding it, and it is far smaller than the certainty of a shared constant. ## Verified | | before | after | |---|---|---| | Podman 6 VM, `--test-threads=1` | **3 failed** | **2 failed** | | Podman 5.7.0 host, full suite | 178/178 | 178/178 | The two that remain are `sibling_resolves_service_by_name_*`, which are #1207 — reproducible with two plain `podman run` commands and no podup involved. ## What this does not fix The load-dependent failures. Those track thread count (3 → 17 of 27 from one thread to eight, measured) and are a different problem, still open on #1322. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…ime's (#1331) Closes #1330, and corrects a measured-false claim in `podman-lane.yml`. ## The test accused podup of the runtime's failure On the Podman 6 VM the message was: ``` service `server` was not reachable by its service name ``` which reads as a podup networking defect. Measured, it was not: | check | result | |---|---| | `podup up` | OK | | alias registered on the network | **`aliases: ['server', ...]`** — podup did its part | | `wget http://server` | `bad address` | | `wget http://<container-name>` | `bad address` — not even the container name | | `aardvark-dns` process | **not running** | | plain `podman run` + `nslookup`, no podup | `connection timed out; no servers could be reached` | Container DNS was down at the runtime level. podup registered the alias, nothing could answer for it, and the test named podup. That cost real time: the failure was read as a podup networking bug, then folded into a netns race in #1207, before measuring showed the DNS server was simply not running. ## The fix is the standard's own rule — assert under the boundary Two layers, asserted separately: - **podup's**: the compose service name appears in the container's network aliases. Checkable with no DNS at all, through a new `test_container_aliases` seam. - **the runtime's**: a lookup for that alias answers. When the alias is present and the lookup fails, the message now says the runtime is what failed. ## Verified both ways Not just that it passes. With the service-name alias registration disabled in `build_per_network_options`, the new assertion fires and prints what it actually found: ``` podup did not register the service name as a network alias: ["75abafa9ddfe"] ``` The old test would have said "not reachable" — indistinguishable from a broken DNS server. ## The lane comment was wrong `podman-lane.yml` claimed: > *measured: on a single-level VM (Podman 5.8.1 and 6.0.1) the same suite never > drops a byte, so the flake is the double-virt socket under concurrent load* Measured on exactly that — a plain KVM guest with Podman 6.0.1 — the suite drops plenty: **17 failures of 27 at eight test threads, 3 at one**, a clean dose-response curve reproducible across repetitions (#1322). Nesting makes it worse; it does not cause it. The comment now says so, because a comment that asserts a measurement nobody re-ran is worse than none. ## Test plan - Both tests green on the host (Podman 5.7.0, working DNS); full suite 178/178. - The alias assertion verified by disabling the control and watching it fire. - `cargo semver-checks`: no update required — the new seam is `#[cfg(feature = "test-helpers")]`, off in the published crate. - fmt and clippy with the CI's own flags. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…un (#1332) Part of #1326. ## The problem, measured | what | lines | |---|---| | `cargo llvm-cov --lib --bins` — what the CI coverage job can see | **79.4%** | | `cargo llvm-cov --all-features` — the full suite | **91.5%** | podup clears the org standard's 90%. It clears it where nothing checks, because the coverage job runs without Podman and every integration test begins `if podman().await.is_none() { return; }`. The `podman-vm` lane is the one place they run. It now measures coverage there and reports it into the job summary. ## What it deliberately does not do **It does not gate.** A threshold set from a number never observed in *this* environment is exactly how a phantom check is born, and this lane is a required check on every pull request. Observe first; gate in a separate change once there are real numbers from real runs. **It does not run on pull requests.** `COVERAGE=1` only for `schedule` and `workflow_dispatch`. A PR must not pay for a second instrumented build inside the VM, nor be able to fail for a reason unrelated to its own change. ## Why it lives in the lane rather than a new workflow A separate coverage workflow would mean duplicating ~200 lines of image fetch, cloud-init seed and qemu boot — and the standard is explicit that CI logic is not duplicated inside a repo. Two copies of that machinery would drift, and the drifting one would be the one nobody watches. ## Verified before pushing - YAML parses. - The substitution yields `COVERAGE=0` for `pull_request`, `1` for `schedule` and `workflow_dispatch` — checked for all three. - The VM's `run-suite.sh` extracted from the seed and passed `bash -n` in **both** modes, because a YAML validator cannot see a shell error inside a heredoc. - The coverage block cannot redden the leg: an install failure reports `install-failed` and the suite's own result is what gates. **And the strongest check is below.** This lane runs on this pull request. If the change broke the PR path, this PR goes red and nothing merges — which is the claim "pull requests are unaffected" proving itself rather than being asserted. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
… observe (#1333) From a mutation round over the self-update path — the most security-sensitive code in podup, since it replaces the running binary in a possibly shared directory. ## Three survivors, all documented hardening | mutation | survived? | |---|---| | temp created `0644` instead of `0600` | **yes** | | `O_NOFOLLOW` dropped | **yes** | | `create_new` (O_EXCL) → `create` | **yes** | | post-install self-test skipped | no — killed | `install_at` is tested, but only for what it produces: contents replaced, created when absent. None of its protections was covered — the authcore pattern exactly, where several dead controls had dedicated tests with the right names. ## But they are unreachable, and I measured that rather than assuming it - **The 0600 mode is overwritten by `write_temp` itself** before it returns: the permission copy at the end sets the target's mode. By the time any caller can stat the file, 0600 is gone. My first test asserted 0600 and failed with `left: 493, right: 384` — which is the code being right and the test being wrong about what is observable. - **`O_NOFOLLOW` and `O_EXCL` close a race.** The `remove_file` above already unlinks a pre-planted symlink — measured with a standalone probe: after it, the link is gone and its victim is untouched. What is left for those flags is the window *between* that unlink and the open, which no in-process test can enter. Their comments now say this where they live, so the next mutation round reads them as reachability rather than as a gap. ## What was reachable, and nothing had pinned it A fourth property in the same function: the permission copy masks setuid/setgid/sticky with `& 0o777`. Without the mask, a target that had been made setuid — by tampering, or by an operator who once did it deliberately — hands the same bit to a binary that has just been fetched over the network. That is a privilege-escalation footgun, and it is observable after the fact. ## Verified by watching it fail Removing the mask turns exactly that test red: ``` a special bit rode from the target onto the new binary ``` The test skips itself if the filesystem did not keep the setuid bit, so it cannot pass vacuously on a mount that strips it. ## Test plan - New test green; the mask mutation kills it and nothing else. - lib 1525, bins 81, fmt and clippy with the CI's own flags. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…the console (#1334) The first lane run with coverage enabled (#1332) reported `install-failed` on **both** legs. The console said why one line earlier: ``` /usr/local/bin/run-suite.sh: line 93: /dev/console: Permission denied PODUP_COVERAGE=install-failed ``` ## The install did not fail — the redirection did `run-suite.sh` runs as `tester` (the runcmd is `sudo -iu tester ...`), which may not open the console device. A denied redirection **aborts the command it was attached to before it executes**, and the shell reports that as the command's own failure. So `cargo install ... >/dev/console` never ran cargo at all. I checked the semantics rather than inferring them from the log: ``` $ if echo RAN-THE-COMMAND >/root/not-writable 2>&1; then echo then; else echo "else rc=$?"; fi bash: /root/not-writable: Permission denied else rc=1 $ ... | grep -c RAN-THE-COMMAND 0 ``` The command leaves no trace of having run, and the `if` takes the else — exactly the shape in the log. The script's other lines reach the log because they **inherit** stdout from the root caller that launched the script; they never open the device themselves. That is why `PODUP_TESTS_RC` and `PODUP_SUMMARY` printed fine three lines above. Both redirects were unnecessary, and both are gone. ## The message was the second defect `install-failed` named the only cause its author imagined, which was not the one that happened. A marker that can describe exactly one failure is a hypothesis wearing the clothes of a diagnosis. The measurement step now has its own marker and dumps what the tool actually said, so the next distinct failure costs a log read instead of another 15-minute VM boot. ## Report-only earned its keep The legs were **green** the whole time this was broken — coverage is observed, never gated, so nothing was blocked by an observation that could not observe. Same run, Podman 5: `178 passed; 0 failed; 0 flaky` in 882s. ## Test plan - YAML parses; the generated `run-suite.sh` extracted from the cloud-init seed passes `bash -n`; zero `/dev/console` redirects remain in it. - Dispatched on this branch, which is the only path that sets `COVERAGE=1`: run `30872834999`. A pull_request run cannot prove this — it runs with coverage off by design. Refs #1326 Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Closes #1330. Reaching a service by its compose name needs three things, and only the first is podup's: 1. podup registers the service name as a **network alias** 2. netavark wires the network 3. aardvark-dns **answers the lookup** Both tests asserted only the end-to-end outcome, so a failure read as a podup defect regardless of which layer broke. On the Podman 6 VM it was layer 3: aardvark-dns was not answering, and nothing resolved — not the alias, not even the container's own name. That message sent me looking for a podup networking bug, then for a netns race, before measuring found the DNS server was down. ## What this adds The alias assertion (layer 1) landed earlier. This is the part that decides **who to blame** when the lookup still fails. It probes the container's **own name**, which the runtime registers with no involvement from podup — so a name podup never touched failing to resolve cannot be podup's alias handling. It runs only on the failure path, so a passing test pays nothing for it. The signatures are the ones measured in #1330, not guesses at busybox's output: a dead server times out, a live server that does not know the name answers NXDOMAIN, and only the first is the runtime's fault. ## The skip is conditional, and that is the whole point A runtime with no DNS cannot answer the question these tests ask, so they skip rather than fail. But **a bare skip would reopen exactly the hole `PODUP_REQUIRE_PODMAN` exists to close** — the lane sets it because a suite reporting `ok` for tests it never ran is worse than a red one. So the skip is conditional on that variable being **absent**. Where the environment promises Podman works, a dead DNS server is a hard failure that names the runtime. ## Verified by mutation, not by reading Neither branch existed before, so neither was covered. Forcing the probe to report DNS down: ``` # without PODUP_REQUIRE_PODMAN skipping: the container runtime's DNS is not answering (aardvark-dns), ... test sibling_resolves_service_by_name_on_shared_network ... ok # with PODUP_REQUIRE_PODMAN=1 test sibling_resolves_service_by_name_on_shared_network ... FAILED the alias `server` is registered, so podup did its part, but the container runtime's DNS server did not answer a lookup for the container's own name either — aardvark-dns is down. ``` Restored, rebuilt, and confirmed the marker is absent from both source and the test binary. ## Why a new file `commands_networking.rs` was at **499 lines of code** against the structure standard's 500 — one line from the ceiling, with no room for the helper. It is now 414; the new file is 105. ## Test plan - Both tests green against local Podman 5.7.0. - Full integration suite: `178 passed; 0 failed` in 121s — the usual baseline. - `cargo fmt --all --check` and `cargo clippy --locked --all-targets --all-features -- -D warnings` with the CI's own flags. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Bumps [clap](https://github.com/clap-rs/clap) from 4.6.4 to 4.6.5. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/clap-rs/clap/releases">clap's releases</a>.</em></p> <blockquote> <h2>v4.6.5</h2> <h2>[4.6.5] - 2026-07-31</h2> <h3>Fixes</h3> <ul> <li><em>(help)</em> Correctly mark which <code>value_names</code> are optional with <code>num_args</code></li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/clap-rs/clap/blob/master/CHANGELOG.md">clap's changelog</a>.</em></p> <blockquote> <h2>[4.6.5] - 2026-07-31</h2> <h3>Fixes</h3> <ul> <li><em>(help)</em> Correctly mark which <code>value_names</code> are optional with <code>num_args</code></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/clap-rs/clap/commit/c8c935504a3f1a015470199ae82c3cb124b5b1d5"><code>c8c9355</code></a> chore: Release</li> <li><a href="https://github.com/clap-rs/clap/commit/af74def7141738c51ace956f58eb32dd94e998ab"><code>af74def</code></a> docs: Update changelog</li> <li><a href="https://github.com/clap-rs/clap/commit/c96f222c35c4ef4bd3ab9927809b2724532a8f6e"><code>c96f222</code></a> Merge pull request <a href="https://redirect.github.com/clap-rs/clap/issues/6368">#6368</a> from truffle-dev/fix/fish-env-escaping</li> <li><a href="https://github.com/clap-rs/clap/commit/49a05cdc99c2151cdd48d5ec4c974151d21c026e"><code>49a05cd</code></a> fix(complete): Two-pass quote fish env-completer</li> <li><a href="https://github.com/clap-rs/clap/commit/e791004a5bbbfe5418fe573a4db777df24d72490"><code>e791004</code></a> test(complete): Snapshot fish env quoting cases</li> <li>See full diff in <a href="https://github.com/clap-rs/clap/compare/clap_complete-v4.6.4...clap_complete-v4.6.5">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… in the VM (#1337) Follow-on from #1334. With the console redirect fixed, cargo actually ran — and said what was wrong all along: ``` error: failed to find llvm-tools-preview, please install llvm-tools-preview, or set LLVM_COV and LLVM_PROFDATA environment variables ``` That line came from the log dump #1334 added, on the first run after it merged. The point of that dump was to make the next failure cost a log read instead of another VM boot, and this is it doing that. ## The cause `cargo-llvm-cov` looks for the `llvm-tools-preview` **rustup component**, and this VM has no rustup — Rust comes from dnf. It accepts the two binaries directly instead, and Fedora's `llvm` package (already in the seed's dnf line) provides both at `/usr/bin`. ## Why the distribution's copy, specifically Measured on a Fedora 45 rawhide guest: | | | |---|---| | `rustc --version --verbose` | LLVM **22.1.6** | | `llvm` package | **22.1.8** | Same series, because both are built for the same Fedora release. A separately pinned LLVM would have no such guarantee, and a profile-format mismatch is the next thing likely to break here — so both versions now print on **every** coverage run, not only when something fails. A mismatch is only diagnosable against the pair of numbers that produced it. ## One marker per path, verified Whoever greps this log gets an answer either way and no reason to suspect a second one, so emitting two would be worse than emitting none. All four paths driven against stubs: ``` sin herramientas -> emitidos=1 PODUP_COVERAGE=llvm-tools-missing install falla -> emitidos=1 PODUP_COVERAGE=install-failed sin linea TOTAL -> emitidos=1 PODUP_COVERAGE=no-total-line camino bueno -> emitidos=1 PODUP_COVERAGE=91.52% ``` ## Test plan - YAML parses; the generated `run-suite.sh` passes `bash -n`. - Dispatched on this branch — the only path that sets `COVERAGE=1`: run `30874830737`. - Coverage stays report-only, so a leg stays green regardless of the outcome. Refs #1326 Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
… suite (#1338) `cargo-llvm-cov` drives its **own** `cargo test` and does not inherit the `--test-threads` cap the gating run uses. Uncapped, the coverage step was measuring a different suite than the one that decides whether the leg is green. ## The measurement Podman 6 leg of run `30874830737` — same code, same VM, same boot: | run | threads | result | duration | |---|---|---|---| | gating | **1** | 178 passed, 0 failed | 1743.03s | | coverage | default | **173 passed, 5 failed** | **441.66s** | The coverage pass was **four times faster**, which is what gives the parallelism away. My first reading was that instrumentation had slowed it into failing — the durations say the opposite, and I would have reported the wrong cause without pulling them. All five failures were `hyper::Error(IncompleteMessage)` — the dropped-connection signature — and **three of them match tests that fail on a local Podman 6 guest running at six threads**: ``` top_skips_a_stopped_service_and_reports_the_rest watch_restart_container watch_sync_creates_missing_target_directory ``` ## What that says beyond this fix Podman 6 still drops connections under concurrency. **The lane is green because it caps threads to one, not because the defect is gone** (#1039, #1104). This is the cleanest evidence of that so far, because it is a controlled comparison *inside a single VM boot* — one variable, same kernel, same podman build — rather than the two-machine comparison that has produced five dead hypotheses in #1207. ## Immediate effect With tests failing, cargo-llvm-cov exits without printing a summary, so the Podman 6 leg reported no number at all (`no-total-line`). Podman 5, whose cap is 2, was unaffected and reported **91.58%** — the first real coverage figure measured where the integration tests actually run, against the CI job's 79.39%. ## Test plan - YAML parses; the generated `run-suite.sh` passes `bash -n` with both placeholders substituted. - `__THREADS__` substitution verified to reach **both** call sites: the workflow's `sed` has no `g` flag, and the two occurrences are on separate lines, so each is replaced. Simulated against the real command — zero left. - Coverage stays report-only, so a leg is green regardless. Refs #1326 Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1341) Closes #1340. Found while bisecting the thread threshold for #1339. A two-thread run reported this, and it took reading the test source to learn what it meant: ``` create_ls::create_makes_containers_without_starting_them assertion `left == right` failed left: 0 right: 1 ``` The assertion is correct. The message cannot say whether `up -d` **failed** or succeeded and the container died, because the line above threw the answer away: ```rust run(&["-f", c, "-p", &proj, "up", "-d"]); // result discarded ``` ## Scope, measured 41 discarded results: | command | discarded | verdict | |---|---|---| | `down` | 21 | teardown — the test is over, fine | | **`up`** | **18** | **everything after depends on it** | | `stop` / `pull` | 2 | left alone | ## Two causes, both fixed **Four identical copies of an unchecked helper.** `cli_flags`, `create_ls`, `niche` and `scale` each defined ```rust fn run(args: &[&str]) -> std::process::Output { Command::new(bin()).args(args).output().unwrap() } ``` which is how they drifted from `stats_flags`'s version — the only one that already asserted. The four copies are gone, replaced by one `run` (unchecked, for teardown) and one `run_ok` (asserts, with the command and its stderr) at the crate root, where every submodule already reaches via `use super::*`. **Local closures that check the wrong thing.** `stats_flags` keeps its checked module-level helper, but three of its tests define a local closure that only `.expect("run podup")`s the *spawn*. A process that starts and exits 1 passes that. Those three `up` calls now assert. ## Verified by making an `up` fail on purpose ``` run_ok → podup [... "up", "-d", "--flag-que-no-existe"] exited exit status: 2: error: unexpected argument '--flag-que-no-existe' found run → left: 0 right: 1 ``` Same test, same failure, two messages. The mutation was reverted and the file confirmed clean. ## Why it is worth doing now This is invisible while the environment is healthy — a green suite never exercises it — and surfaces exactly when something else is already broken, which is when the diagnosis is worth the most. Today it turned "is podup's `up` failing under concurrency?" into an assertion about a count. Same shape as #1330, where a DNS test blamed podup for the runtime's dead resolver. A test that discards the evidence of its own precondition reports the wrong thing with total confidence. ## Test plan - Full integration suite: `178 passed; 0 failed` in 120.56s. - lib 1525, bins 81. - `cargo fmt --all --check` and `cargo clippy --locked --all-targets --all-features -- -D warnings`; the two imports orphaned by deleting the duplicate helpers are removed, so the build is warning-free. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1342) Capping the coverage run to the gating run's thread count (#1338) means the Podman 6 leg now runs **two serialised suites**. Measured on run `30879785232`: | leg | started | gating suite done | coverage done | total | budget used | |---|---|---|---|---|---| | Podman 6 | 05:08:53 | 05:40:27 | 06:15:13 | **66m27s** | **89%** of 75 | | Podman 5 | 05:08:58 | 05:26:15 | 05:45:20 | 36m28s | 49% | It has not failed, which is the only useful moment to change this. A slower runner or a heavier rawhide image tips it over, and **a job killed on the timeout tells you nothing about the coverage it was measuring** — the same reason the timeout was raised for the one-thread experiment in #1039. 95 rather than 90 so a bad-luck run still lands. Costs nothing on the pull-request path: coverage is off there by design, and the leg finishes in roughly half the time. ## Also, the number arrived Same run, both legs, with #1337 and #1338 in place: | leg | coverage | |---|---| | Podman 5.8.1 | **91.56%** | | Podman 6.0.1 | **91.59%** | Three samples now sit within 0.03pp of each other, against the CI job's 79.39% and the standard's 90%. Podman 6 previously reported `no-total-line`; the thread cap fixed it, which is the confirmation #1338 was waiting for. ## Test plan - YAML parses. - Nothing else in the file changes — this is the `timeout-minutes` value and the comment recording why. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…ead of failing (#1343) The fix #1339's step 3 pointed at, and it is podup-side, so it waits on nothing upstream. ## What the measurement said Instrumenting the suite at three threads (`RUST_LOG=podup=debug`, every libpod request logged) showed the drops land on **state-changing POSTs** — `exec`, `restart`, `stop`, container `DELETE` — never on the reads. And the timestamps inside one failing test's own captured block: ``` 06:24:06.700679 POST …/containers/t3812-rsr-worker-1/restart 06:24:16.826346 POST …/containers/t3812-rsr-worker-2/restart ← 10.1s later panicked … Podman(Hyper(hyper::Error(IncompleteMessage))) ``` Everything else in that trace runs in **milliseconds**. The gap is the first restart burning its full stop grace; the drop lands on the request after it. Two explanations that suggested, both ruled out by reading the client rather than guessing: | candidate | why not | |---|---| | a client deadline | `READ_TIMEOUT` 120s, `CONNECT_TIMEOUT` 30s — nowhere near 10s | | a keep-alive race on a pooled connection | there is no pool; `http1::handshake` runs per request on a fresh socket | So the server severed the response, and the transport cannot say whether the operation ran. That is #1104's undecidable question, arriving from the other end. ## The fix is the pattern podup already has `cp` verifies the destination entry moved (#1097); `stats` re-checks the running set (#1080). `run_lifecycle_op` now does the same — and because `start`, `restart` and `kill` already share it, one change covers all three. A `LifecycleGoal` names what the operation was for, and **only a container that reached it counts as success**. Both other shapes fail closed: - reached the goal → success, with a `warn!` recording that the response was lost - did not reach it → the original error - re-check unreadable → the original error ## The harness could not produce this shape at all `FakeReply`'s four variants all send *something*, and `is_incomplete_message` is about the message **head** — so nothing in the tree could reach that discriminator, **including the one `cp` has relied on since #1097**. `ClosedWithoutResponse` fills that: request accepted, connection closed, no status line. ## Verified by mutation, not by reading | mutation | result | |---|---| | remove the re-check branch | the **2 success tests** go red | | make the re-check always agree | the **did-not-reach-the-goal** test goes red | The unreadable-re-check test stays green under the second, because it lives in a different arm — so the tests tell the two fail-closed shapes apart rather than conflating them. ## Test plan - lib 1530, bins 81; full integration suite `178 passed; 0 failed` in 120s. - `cargo fmt --all --check`, `cargo clippy --locked --all-targets --all-features -- -D warnings`. - `commands.rs` at 478 code lines, under the standard's 500; the new tests are their own file rather than pushing it over. Refs #1339 Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…asured on (#1344) Stacked on #1343 — its commit shows here until it merges, after which this diff stands alone. #1339's measurement named four state-changing calls. #1343 covered `restart`, `start` and `kill` (all three share `run_lifecycle_op`). This covers the other three: **`stop`, container `DELETE`, and `exec`** — the last of which the measurement named *most*, twice of six. | endpoint | how it is resolved | why | |---|---|---| | `stop` | re-check → `NotRunning` | own call site, not via `run_lifecycle_op` | | `rm` | re-check → **`Gone`** | absence, not merely stopped | | `exec` create | **retry once** | the lost thing is the exec id, not a container state | ## `rm` needed a goal of its own `NotRunning` is satisfied by a container that stopped but is **still there**, which would read a failed removal as a success. Mutating `Gone` to behave like `NotRunning` turns two tests red, so the distinction bites. ## `exec` could not use the same trick, and does not A lost create response leaves no id to ask about, and a container running several execs at once cannot say which of its `ExecIDs` was this one. Retrying works instead, **because creating an exec changes nothing** — measured on Podman 5.7.0 rather than assumed: ``` 5 execs created and never started container: running, same pid ExecIDs: 5 processes: no extra process inside ``` Inert handles that die with the container. The worst case of a retry is one leaked handle, against an `exec` that currently fails for a reason that was never about the command. **Once only** — a second drop is a daemon problem, not a transient, and an unbounded retry turns a broken socket into a hang. **Local to `exec`** — the same retry in the client would apply to container create, where it means two containers. ## Two holes the tests found in themselves - The fake dropped only `POST`s, so the container `DELETE` fell through to the 404 arm and was read as an idempotent no-op — **a removal test measuring nothing**. It now drops both methods. - The first `exec` test drove `test_exec_capture`, which **builds its own request and never reaches this code**. It would have passed while testing nothing. The test drives `exec_with_options` now. ## Where the code lives The re-check moved out of `commands.rs` into `drop_recheck.rs` with the goal enum and the state lookup. `commands.rs` was at **493** code lines against the standard's 500; it is **424** now. ## Verified by mutation | mutation | result | |---|---| | `Gone` behaves like `NotRunning` | pure test + merely-stopped wire test red | | `stop` loses its re-check arm | its success test red | | `exec` retry removed | both exec tests red | | `exec` retry loops instead of running once | the bounded-retry test red | ## Test plan - lib 1537, bins 81; full integration suite `178 passed; 0 failed` in 120s. - `cargo fmt --all --check`, `cargo clippy --locked --all-targets --all-features -- -D warnings`. - `exec.rs` 472 code lines, `commands.rs` 424 — both under the standard's 500. Refs #1339 --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
A **patch** release. `cargo semver-checks` against `origin/main` agrees: ``` Checking podup v3.6.0 -> v3.6.1 (patch change) 223 checks: 223 pass, 30 skip Summary no semver update required ``` ## What it carries One user-visible fix, in two halves — everything else in the window is CI and test work with no runtime effect, plus a clap patch bump. **A lifecycle operation whose response the daemon drops was reported as a failure even when it had completed.** Podman 6 severs exactly these responses under concurrency; measured on the lane, where the same suite passes 178/178 at one test thread and fails 5 at the default, all dropped connections. So `up`, `down` and `restart` on a multi-service project could fail for a reason that was never the command. `restart`, `start`, `kill`, `stop` and `rm` now confirm the container reached the state the operation was for, and only then report success — a container that did not reach it, or a state that cannot be re-read, still fails. **`exec` retries once** instead, because a dropped create loses the *session id* rather than a container state, so there is nothing to re-check. Measured before relying on it: an exec created and never started allocates no process and is discarded with the container. ## Blocked on #1344 The changelog describes `stop`, `rm` and `exec`, which are in #1344 and not yet on develop. **This must not merge before that does** — a changelog describing behaviour that did not ship is worse than a thinner one. If #1344 does not land, the entry gets trimmed to what #1343 alone delivers. ## Version in all three files `Cargo.toml`, `Cargo.lock` and `debian/changelog`. Missing the changelog ships .debs carrying the previous version and strands apt users on the one upgrade path the release key forces them onto. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added
release
type:chore
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A patch release.
cargo semver-checksagainstorigin/main:What ships
One user-visible fix, in two halves. Everything else in the window is CI and
test work with no runtime effect, plus a clap patch bump.
A lifecycle operation whose response the daemon drops is no longer reported as
a failure when it actually completed. Podman 6 severs exactly these responses
under concurrency — measured on the lane, where the same suite passes 178/178 at
one test thread and fails 5 at the default, every failure a dropped connection.
So
up,downandrestarton a multi-service project could fail for a reasonthat was never the command.
restart,start,kill,stopandrmnow confirm the container reached thestate the operation was for, and only then report success. Both other shapes
still fail: a container that did not reach it, and a state that cannot be
re-read.
rmrequires absence rather than merely not-running, so a failedremoval cannot pass as a success.
execretries once instead. A dropped create loses the session id, not acontainer state, so there is nothing to re-check. Measured before relying on it:
an exec created and never started allocates no process and is discarded with the
container, so the worst case is one leaked handle.
How it was verified
Every control here was checked by deleting it and watching a test go red —
removing the re-check, degrading
rm's goal toNotRunning, removing theexecretry, and making that retry loop instead of running once. Two holes inthe tests themselves surfaced that way: a fake that dropped only POSTs (so the
container DELETE was read as an idempotent no-op) and an
exectest driving ahelper that never reaches the code under test.
Merge with a merge commit
Not a squash — main keeps the history.