Conversation
## Summary - Nothing reported that `main` or `develop` was failing its own gates. - The freshness watchers cannot see it: `reusable-schedule-freshness` reads the newest **successful** scheduled run of the workflow it names, so a failing run is missing from its answer by construction. - Fixes #1719. ## Why it matters more here The three distribution channels hit this on 2026-09-06: a pull request merged while its suite was still reporting, `main` red in two of them for twenty minutes, and the only reason anybody knew is that somebody went looking. Here the release is cut from `main`, so a red `main` is the published state failing its gates rather than a broken integration branch. ## Changes - New `.github/workflows/reusable-branch-health.yml`: reads the newest COMPLETED run of a workflow on a branch and fails when its conclusion is not `success`. `status=completed` is what excludes a run still in progress, and a comment says so, because that is the parameter somebody later "simplifies" away. - New `.github/scripts/check-branch-conclusion.sh`: the conclusion logic, extracted so it can be exercised against planted API answers with no `gh` and no network. - `ci.yml`: two jobs, `health-main` and `health-develop`. They stay separate because each check name carries the branch it watches, and a stable check name is what a required check depends on. - `lint-shell.yml`: runs the new shell test, and installs `jq`. `cancelled` and `skipped` are named rather than folded into a generic failure. Neither is a pass: a cancelled run is a suite that was interrupted, and a skipped one is the workflow deciding not to exercise the gate at all. An empty history fails with its own message, because a protected branch with no completed run is not a branch to release from. **Neither job may become a required status check.** A red branch would then block the pull request that repairs it, which is the failure this exists to surface. The reason is written twice, in `ci.yml` beside the jobs and inside the reusable, and both copies say not to remove the other. ## Tests - `tests/shell/branch-health-conclusion.test.sh`: sixteen assertions over six cases plus a negative control, driving the real script against planted API answers. `success` passes; `failure`, `cancelled`, `skipped` and an unknown conclusion each fail with their own message; an empty list reports no run on record. - `tests/workflow_branch_health.rs`: five tests over the workflow shape, with a planted YAML negative control proving the parser reads what it claims to. Signed-off-by: Jose <75870284+Jaro-c@users.noreply.github.com>
`install.sh` pins the protocol across redirects and nothing exercised it: `--proto-redir` appeared three times in the installer and zero times under `tests/`, so deleting the flag left every suite green. The new case stands up two local listeners, an https origin answering 302 to an http URL and an http target serving the fixture's real artifact bytes, and drives the installer's own `download` at the https origin. The target has to serve a VALID payload: with bad bytes the signature check refuses them first and the case passes for the wrong reason. The refusal is attributed rather than counted, so a non-zero exit for an unrelated reason does not satisfy it: the case asserts the failure came from the download step and that nothing was written. I also corrected the comment above the curl call, because it says something I measured to be false. It claimed `--proto '=https'` only restricts the initial URL and that the redirect needs its own pin. It does not: curl's manual says protocols denied by `--proto` are not overridden by `--proto-redir`, and on curl 8.18.0 and 7.81.0 the redirect to http is refused with the flag and without it. The flag stays, because the bare spelling `--proto https` adds rather than restricts and under that form `--proto-redir` is the only lock, but the comment now says what is true. This means the case cannot be pinned by deleting `--proto-redir` alone, which is what I asked for when I wrote the issue. I ran that deletion and the case stayed green. What it does pin is the end-to-end refusal, so it fails the day someone writes `--proto https` without the `=`. Closes #1722. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Two things the freshness block did not say. The first is that these jobs are advisory. A `freshness-*` job fails because a cron somewhere else stopped firing, which has nothing to do with the change under review, so promoting one to a required check would block every pull request for the length of an unrelated outage. That rule lived only in the organisation standard, which is not the file open in front of somebody wiring a ruleset. It is now beside the jobs. The second is `podman-lane-develop-nightly.yml`. It declares a cron and has no watcher, and nothing said whether that was decided or forgotten, so a reader had to guess. It is a decision, and the comment now carries both halves of it. I went to add the watcher and measured why it cannot go in yet. The guard reads the newest scheduled run with `status=success`. On 2026-09-06 the last successful nightly was 2026-09-01: five consecutive nights failed the 88 per cent coverage floor, at 85.51, 85.38 and 85.40 on the three runs that print the figure. Any threshold under five days is red on arrival. A cron that fires nightly and fails nightly looks exactly like a cron that stopped, which is the one thing this guard cannot tell apart. The lane also produces an observation rather than a gate: the threshold a pull request answers to is the 76 per cent on `rust / Coverage`, so a watcher would be reporting the staleness of a number nobody is required to keep fresh. The entry belongs here once the lane is green again. Closes #1723. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
The tooling-isolation step found a secret with a regex that only matched `secrets.NAME`, and it searched the re-serialized job alone. Two shapes hold a secret at runtime and walked past it: the index form `secrets['NAME']`, and a workflow-level `env:`, which is the normal place to put a value several jobs need. The scan now matches all three spellings, whitespace inside the brackets included, and counts a job as holding a secret when the workflow-level `env:` holds one. Each violation carries where the secret came from, so an author who reads the message, looks at the job and sees no secret is told to look further up the file instead of concluding the check is broken. `secrets: inherit` and a `secrets:` block on a reusable call still do not count, and the case pinning that stays green. The new test extracts the step's `run:` body from the workflow, so it exercises the script as it ships, and runs it against fixtures for both new shapes, the already caught form as the positive control, and a job with no secret that installs tooling as the negative one. Each refusal asserts which message fired. I planted both shapes and watched them go red before wiring the test in. Closes #1720. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Renaming a job id, or the `name:` of the job inside the reusable it calls, changes the status check GitHub emits. The ruleset still requires the old name, the check is never reported, and the pull request sits BLOCKED with nothing saying why. Nothing in the tree asserted that the jobs behind the required names still exist under those names: I grepped `Supported Podman majors`, `rust / Coverage` and `develop-only` under `tests/` and got nothing. The new test pairs each required name with the job id and the job name that produce it, and fails when one no longer resolves. The list is a literal, because a list derived from the tree cannot notice a job that was removed: the derivation removes the name at the same time. The comment on it says it has to move with the rulesets. `main` and `develop` are separate lists rather than one, so the asymmetry survives: `main-guard.yml` only triggers on pull requests into `main`, so requiring `develop-only` on `develop` would block every merge there with a check that is never reported. Two corrections to what I wrote in the issue, both measured with `gh pr checks --required` against a merged pull request on each branch, deduplicated: - `rust / Format` does not exist. `reusable-rust-ci.yml:75` reads `name: Format & lint` and has since #1566. I encoded what GitHub emits and said so in the test. - The counts are eleven on `main` and ten on `develop`, not twelve and eleven. No name is missing from the list: the count was wrong. Verification is in the tests themselves: three planted renames, a negative control against a tree where nothing resolves, and a control proving the reader reads job names rather than the prose around them. I also renamed `Coverage` in the real tree and watched it report `rust / Coverage: no workflow pairs job id 'rust' with a job named 'Coverage'`. Closes #1721. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
The tooling-isolation step found a secret with a regex that only matched `secrets.NAME`, and it searched the re-serialized job alone. Two shapes hold a secret at runtime and walked past it: the index form `secrets['NAME']`, and a workflow-level `env:`, which is the normal place to put a value several jobs need. The scan now matches all three spellings, whitespace inside the brackets included, and counts a job as holding a secret when the workflow-level `env:` holds one. Each violation carries where the secret came from, so an author who reads the message, looks at the job and sees no secret is told to look further up the file instead of concluding the check is broken. `secrets: inherit` and a `secrets:` block on a reusable call still do not count, and the case pinning that stays green. The new test extracts the step's `run:` body from the workflow, so it exercises the script as it ships, and runs it against fixtures for both new shapes, the already caught form as the positive control, and a job with no secret that installs tooling as the negative one. Each refusal asserts which message fired. I planted both shapes and watched them go red before wiring the test in. Closes #1720. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Eleven status checks are required on `main` and ten on `develop`, and every one is emitted by a job that a job-level `if:` can skip. GitHub reports a skipped required check as success, so one line in a pull request switches off coverage, the MSRV floor or the validation on both supported Podman majors, and the ruleset still reports the gate as satisfied. Nothing here asserted otherwise: this repository's `reusable-workflow-lint.yml` carried no assertion of that kind at all. The rule is not "no conditional on a caller", which is the shape the channels prove and which would refuse this tree on day one: `podman-lane.yml` carries `if: always()` on `podman-majors`, the job that emits `Supported Podman majors`, and that is the gate pattern from What matters is whether the conditional can SKIP the job. `always()` never evaluates false, so the job runs and the check is reported. Anything that can be false skips it, and a skipped required check reports success. The wrapped spelling of `always()` is the same expression to GitHub and is accepted too, while `always()` with a tail keeps its tail, can still be false, and is refused. The scan skips `reusable-*` files. The eleven conditionals inside the reusables are how a reusable is configured, not a switch on a caller, and eight of them are in `reusable-rust-ci.yml` alone. The check names come from `gh pr checks --required` against a merged pull request on each branch, deduplicated. That is also where the two errors in what I wrote in the issue come from: the counts are eleven and ten rather than twelve and eleven, and `rust / Format` does not exist, since the job is named `Format & lint`. 28 assertions, each refusal paired with an acceptance of the same shape, the real tree as the positive control, and a seam either side: a switching conditional inside a reusable stays allowed, and so does one on a job that emits nothing required. One note for whoever edits this scanner next. A literal expression delimiter written inside the `run:` block is parsed by actionlint as an expression of this workflow and fails the lint, so the one this code needs is built by concatenation and says why. Closes #1718. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Two user-visible fixes to `audit` landed since 5.9.1 and no published binary carries them, so they go out as a patch. All three files that stamp a version move together: `Cargo.toml`, `debian/changelog` and `Cargo.lock`. The release workflow refuses to build when they disagree, and the reason is 1.9.0, which shipped `podup_1.8.0_*.deb` because only `Cargo.toml` had been touched: apt saw the version it already had and reported nothing to upgrade. The changelog entry is what the two fixes do to somebody running the tool, not the shape of the code that does it. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
`podup cp` container to host moved the operator's files out of a directory that was never part of the copy. `flatten_single_wrapper_dir` called `is_dir()` on the extracted wrapper, `is_dir()` follows symlinks, and a compromised container can plant one: the flatten step then read the target's contents and renamed them into the destination. Reproduced end to end with the real binary, with `~/.ssh` as the target: the directory ends empty and the error surfaces after the move. The check now goes through `symlink_metadata` at both sites, so a symlink is a symlink and not the thing it points at, and a destination that is itself a symlink is refused before anything touches the filesystem. The trigger I wrote in the issue was wrong, and the test that came with it passed on the unfixed tree: a destination that is a symlink returns at the `dst.is_dir()` branch above and never reaches flatten. The symlink has to arrive out of the tar. The test here builds that archive in memory, plants a victim directory beside the destination, and asserts the victim still holds its files. On the tree before this change it fails with `id_ed25519 was moved out of the victim directory through a symlink planted in the archive`, and reverting only the flatten half brings that failure back. Closes #1736. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
`asset-contract.yml` pinned `actions/checkout` to a 39-character SHA.
GitHub answers 422 for any other length, so the job never started, and
that job is `shell-proto-redir`: the only thing exercising install.sh's
refusal of an https to http redirect, which landed the same day.
Nothing noticed because the shell-suite checks assert that every test is
NAMED by a workflow. Being named is not being run, and a broken pin
leaves the naming intact, so the suite stayed green over a fixture that
had never executed.
Two changes. The pin is corrected to the real 40-character SHA for the
version it already claimed. And the workflow lint now refuses any
`uses:` pin that is not exactly 40 hexadecimal characters, with a
message that says GitHub answers 422 and the job never runs, so the
author is not left guessing why a check went missing. That second part
is the one that generalises; the first is a typo.
The check was run against the tree before the pin was corrected, which
is the only moment that proof was available:
FAIL the real repository tree passes the pin-length assertion
ok a 39-character pin is refused
31 assertions, each refusal paired with an acceptance of the same shape,
the real tree as the positive control, and a case for an unreadable
workflow so a parse failure is a warning rather than a silent skip.
Closes #1741.
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
`count_alias_refs` toggled its "inside a quoted scalar" state on every
`'`, wherever it sat. In `[don't, *a, *a, ...]` that toggle flipped on
the apostrophe of a plain scalar and every `*a` after it was treated as
quoted text. The count came back zero, the `refs == 0` early return
fired, and BOTH guards were skipped: the reference cap and the
document-size cap. Two files seven bytes apart, measured on develop:
ctrl2.yaml (24101 B) refused in 0.10 s, 20 MB
apos2.yaml (24108 B) accepted, guards never consulted
With this change both are refused identically.
A quote now only opens a scalar at a value position, so an apostrophe
inside a plain scalar is what YAML says it is: an ordinary character.
`#` gets the same treatment, since `x#y` is not a comment either.
The reference count was also the wrong quantity to bound. It says
nothing about how much memory the expansion takes: a small anchor
referenced many times across a few hundred kilobytes reaches gigabytes
while staying under a per-reference cap. The new bound is on expanded
size, `refs * content.len()`, and the refusal says how many bytes the
expansion would reach rather than only how many references it counted.
`interpolate_scalar` re-parsed `.env`-supplied text with no guard at
all, so text arriving that way bypassed both caps. It goes through the
same guard now.
The nested-anchor multiplication the issue also claimed is not real:
the library bounds jumps at `events.len() * 100`, and nesting
multiplies the jump count as fast as it multiplies nodes, so eleven
shapes of 288 to 763 bytes either finished under 10 MB or were refused
in 0.10 s. The hole needs the opposite input, a few hundred kilobytes
that is flat, which is what the expansion cap addresses and what the
figures above are built from.
`count_alias_refs_ignores_quotes_comments_and_globs` stays: it pins
that legitimate files are not over-counted, and it is now paired with
cases for the other direction, which is the direction nothing checked.
Closes #1737.
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
`escape_unit_value` exempted `PodmanArgs=` on the grounds that we render
and quote that line ourselves. True for `Exec` and `Entrypoint`, false
for `PodmanArgs`: seven sites interpolated raw compose strings into it,
so one `security_opt` entry put `--privileged --net=host -v /:/hostfs`
into the argv Podman's own generator builds. Host root, bind-mounted
into a privileged container on the host network, from a compose file.
`quote_podman_arg_value` now wraps each interpolated value at those
seven sites, strips control characters and doubles `%`, which that path
was also missing: an injected `-v %h:/homedir` was expanded by systemd
into the operator's home. `Environment=` already did this and is what
the helper follows.
Measured against Podman 5.7.0's generator on the file from the issue,
after the change:
"--cpuset-cpus=0\x20--privileged\x20-v\x20/:/hostfs2"
--security-opt
"apparmor=unconfined\x20--privileged\x20--net=host\x20-v\x20/:/hostfs"
One argv element each. No loose `--privileged`.
The scope stays exactly this key. Seventeen other string fields already
came out as a single quoted element and are untouched.
The tests assert the argv Podman would build rather than the unit text,
because the unit text is what looked correct for the whole life of this
bug. Making `quote_podman_arg_value` return its input unchanged turns
eight of them red, including the two named for the smuggling itself, and
the tree is green with it restored.
`docs/security-model.md` claimed Quadlet values are filtered through
`escape_unit_value`. That sentence was false for this key and now names
both helpers.
One acceptance criterion I wrote in the issue was wrong and is not
implemented: it asked for the `privileged` audit finding to fire on the
smuggled spelling. After this change the smuggled spelling grants no
privilege, so reporting one would be a false positive. Measured: the
honest `privileged: true` file still reports `privileged`, and the
smuggled file reports the same six findings without it, which is now the
correct answer rather than the silence the issue complained about.
Closes #1734.
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
---------
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
) `podup logs` stopped at about 1 MiB and exited 0. An operator reading logs to diagnose an incident got a short answer and a success code, which is worse than an error because they act on it. `total_received` was a running tally of everything the stream had delivered rather than a bound on the buffer still owed to the parser. Frames were split off the front of `buf` and the counter never learned they were gone, so a stream of many small frames tripped `StreamTooLarge` around a thousand frames in regardless of how much was actually held. The counter now subtracts what each frame consumed, which is what the sibling `parse_json_lines` already did on `take_json_line`. The cap itself stays: it is the safety net against a daemon-controlled allocation on a slow consumer, and per-frame overruns are still caught one level down by `parse_frame`. `parse_multiplexed` keeps its public shape and gains a generic body form underneath it, so the accounting can be driven against an in-memory body instead of a Podman socket. Dropping the subtraction turns the new case red on its own, and the full suite is green with it in place. Closes #1739. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1754) Repeating a variable during interpolation had no ceiling. Output grows by the number of references times the length of the value, so a few hundred kilobytes of input where one variable repeats reaches gigabytes before anything else has a chance to fire: 300 KB in, a 3.27 GB allocation, SIGABRT in 0.97 s. The bound is checked before each `push_str` rather than after the pass, so the refusal happens before the offending allocation instead of reporting it once the memory is already gone. The message names the variable and the size it reached. A long secret or a multi-line config blob is a legitimate reason for a large value, and an operator who hits this needs to tell that apart from a hostile document. This is a different path from the YAML alias caps in #1737, which is why that change does not close this one: the alias guards bound the document's expansion, and this bounds what substitution writes out of it. Forcing the comparison false turns the refusal case red and leaves the legitimate-repetition case green, which is the pair that says the cap is doing the work rather than the test passing for its own reasons. Closes #1738. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…us capabilities (#1756) Two different defects, not three instances of one, which is why this does not use a single mechanism the way I asked for in the issue. Drift, closed by reading the resolved value. `no_memory_limit` asked whether `mem_limit` was set; the engine parses it, and `parse_memory` returns `None` for junk. So `mem_limit: not-a-size` produced no limit at runtime while `audit --strict` called the service constrained. Both checks now read through the same parser the engine uses, and `no_new_privileges_off` reads the resolved `security_opt` rather than the compose-side text, so an entry that cancels the protection is seen. A missing check, which reading the resolved value would not have fixed. `cap_add: [SYS_MODULE, ...]` is honoured exactly as written, so nothing has drifted: what was absent is an opinion about which capabilities are dangerous. `dangerous_capability_reason` is that opinion, and it carries the reason for each entry rather than a bare list, because the next person to edit it needs to know why `MKNOD` is on it and `CHOWN` is not. Verified by breaking each half separately: removing `SYS_MODULE` from the list turns the curated-list case red, and making the memory check look at presence again turns the unparseable-limit case red. Each sabotage takes down its own case and leaves the other three green. `audit_dangerous_capability_passes_when_empty` used `NET_ADMIN` as its safe sample, which is no longer a safe answer, so its sample moved. Closes #1743. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…rst (#1757) Repeating a variable during interpolation had no ceiling. Output grows by the number of references times the length of the value, so a few hundred kilobytes of input where one variable repeats reaches gigabytes before anything else has a chance to fire: 300 KB in, a 3.27 GB allocation, SIGABRT in 0.97 s. The bound is checked before each `push_str` rather than after the pass, so the refusal happens before the offending allocation instead of reporting it once the memory is already gone. The message names the variable and the size it reached. A long secret or a multi-line config blob is a legitimate reason for a large value, and an operator who hits this needs to tell that apart from a hostile document. This is a different path from the YAML alias caps in #1737, which is why that change does not close this one: the alias guards bound the document's expansion, and this bounds what substitution writes out of it. Forcing the comparison false turns the refusal case red and leaves the legitimate-repetition case green, which is the pair that says the cap is doing the work rather than the test passing for its own reasons. Closes #1738. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
… image once (#1755) Three N+1s on the same axis, each with a counting test rather than a timing one, because a timing assertion is flaky and does not say what regressed. `podup top` asked for the project's containers once per service: forty services meant forty listing GETs where `ps` makes one. It now shares the same bulk path `logs`, `port`, `exec` and `cp` already use from #1445, so the fix follows a shape this tree already had rather than inventing one. A service missing from the bulk map yields no names, which keeps the per-service helper's contract from #1250 that there is no fallback to static names. Warm `up` inspected the same image tag once per replica: twenty-one identical GETs where one would do. `podup images` inspected serially and without deduplication. The tests count requests. Moving the bulk fetch back inside the loop makes the `top` case report `left: 4, right: 1` and print the four identical listing URLs it saw, so the failure names the defect instead of only the number. Closes #1742. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Three things about `--connection-pool-size` that only made sense
together, so they land together.
The guard was released when `send` returned, before the body was read.
With `Content-Length` that is harmless: the body is delimited and the
wire is never shared. With chunked encoding the wire IS the delimiter,
so a second caller taking the connection mid-body reads another
response's bytes. `Client::send` now returns a `BufferedResponse` that
owns the guard, so it cannot be released until the body is done.
The flag was documented as "Default: 8" and defaulted to 0, which meant
every buffered call opened a fresh socket. The documentation was right
about the intent, so the code now matches it: the pool is on, and
`--connection-pool-size 0` opts out.
Those two had to move together. Fixing the default alone would have
shipped the corruption to everybody.
The pool's own tests could not see any of this: the harness sends
`Content-Length`, which is the one framing where releasing early does
no harm. The new test serves a chunked body with the headers first and
the body after a delay, so eight tasks race for a one-connection pool.
Dropped into an unmodified `develop` it fails:
chunked body corrupted: parse failed:
Hyper(hyper::Error(Canceled, "connection was not ready"))
and passes here. The existing `Content-Length` cases stay, because a
harness that only exercises the safe framing is why this survived.
The environment variable was spelled `PODUP_LIBCOD_POOL`, a typo of
`LIBPOD`, in all six places. It is now `PODUP_LIBPOD_POOL`, and the old
name is read as a fallback when the new one is unset, so a script that
exports the old spelling keeps working.
Closes #1740.
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
---------
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Seven entries, each with its own case built from an input that behaves
wrongly on the tree before this change.
A refused rollback install left a root-owned executable in
`/usr/local/bin` while printing that the staged file had been removed.
The message was false at the moment it was printed.
Terminal-escape injection reached five sinks, `audit`'s own detail lines
among them, and the escapes survive a pipe. `sanitize_cell` escapes
control characters before width is measured, not after, so an escaped
cell cannot overflow its column either. Making the helper return its
input unchanged turns the two audit cases red.
`extends: {file:}` was re-read and re-parsed once per referring service.
The issue called this linear and put it at hours of CPU; it is not. At
the 16 MiB file cap that already exists, twenty referring services abort
on allocation in 26 seconds at 5.8 GB, so the bound belongs on expanded
bytes and the parse is cached.
Two lists answered "which host-binding modes matter" and had drifted:
the one wired to `--strict` missed the `container:<id>` form the runtime
detector already warns about, so the two disagreed about the same file.
One list now, both callers. Making the prefix never match turns
`audit_host_namespace_flags_network_mode_container_id` red and leaves
the other six green.
`podup images` inspected serially without deduplication, the
build-context walk enumerated ignored directories before filtering them,
and `internal/compose/mod.rs` skipped diagnostics for `-f -`, so a piped
file with `memroy:` got neither the limit nor the warning. The existing
coverage for that spelling tested the helper and bypassed the
integration that skipped it.
Closes #1746.
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
---------
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1761) An independent review of #1758 found three claims in this file that the code does not support. All three are mine, from #1740. `accepted() >= 1` was labelled "at least one connection was opened and reused". A client that opens a fresh socket for every call satisfies it, so the assertion held down nothing. It is now bounded from above too: 160 round-trips through a cap-of-one pool must open strictly fewer than 160 sockets. Releasing the guard early and reacquiring, which the old assertion passed, now reports `281 sockets accepted for 160 requests`. That sabotage looked inert when I ran it during review; it was not, the assertion could not see it. `expect("chunked body corrupted: parse failed")` labelled every error as corruption, including a hyper dispatch cancellation (`Canceled, "connection was not ready"`), which is a different defect tracked in #1758. A failure message that names the wrong cause makes the test a source of false diagnosis. It now says only that the request failed, and says why it does not guess. The header comment said `Content-Length` framing makes releasing the guard early harmless. It does not: a declared length large enough or delivered slowly enough leaves a body in flight exactly as a chunked one does. What the old harness lacked was a body still arriving when `send` returned, which is a property of that harness rather than of the framing, and the comment now says that instead. No behaviour changes here. The test asserted less than it claimed, and now asserts what it claims. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Nine entries, the two security ones first. `extract_archive` routed on `Path::is_dir`, which follows symlinks, the same class #1736 closed one level down. A destination that is a symlink is now classified as a symlink through `symlink_metadata`. Swapping the call back to `metadata` turns `cp_destination_kind_treats_a_symlink_to_a_directory_as_a_symlink` red and leaves the other forty-six copy cases green. `write_temp` used `O_NOFOLLOW` and `O_EXCL` on Unix and plain `File::create` elsewhere, so a junction planted in a Windows install directory could redirect verified bytes. The Windows branch now mirrors the Unix invariants with `create_new(true)` and `FILE_FLAG_OPEN_REPARSE_POINT`, so the open is given the reparse point itself rather than what it points at. `post_hijack` had no connect or read timeout: a stuck or absent peer hung the command with no ceiling. Both now bound, and the head read at 30s rather than the full 120s, since a status line does not need two minutes. `install.sh --help` was broken through the exact pipe the script documents, `env_file:` on a FIFO blocked forever, `up` fetched the project container list twice with a byte-identical filter, `watch` built its sync tar on the async executor, `autostart` in quadlet mode forked one `systemctl` per service, and the key-copies control checked slot 0 only, which is fine until a rotation starts. `tokio`'s `test-util` feature is added as a dev-dependency so the timeout cases can assert wall-clock ceilings with a paused clock instead of making the suite wait thirty real seconds. Closes #1747. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1765) The pool treated "guard returned" as "hyper ready and reusable". It is not. Hyper publishes body completion from its read path and only then polls its write path, announcing readiness afterwards, so another worker on a multithreaded runtime can observe completion, take the connection and send before that announcement. `send_request` then fails with `Canceled, "connection was not ready"` on a socket that is perfectly healthy, which is why #1740 holding the guard across the body read did not close this: exclusive ownership is not synchronisation with hyper. `acquire` now awaits `SendRequest::ready()` before handing an idle connection out, and a connection whose `ready()` errors is replaced rather than returned. That also covers the second half of the contract: an idle connection that closed while parked no longer reaches a caller. The reproduction is a cap-of-one sequential test rather than the live Podman lane, which is what an independent trace suggested and what turned out to be true: the sequential case catches it and the concurrent one does not. Eight worker threads, a chunked fixture with 500us between headers and body to open the window, and 5000 iterations. On the unfixed tree it fails around iteration 90, consistently. Making the `ready()` call unreachable reproduces the issue's exact message: request 3521 saw `Canceled, "connection was not ready"` on a healthy socket A `current_thread` runtime hides the defect entirely, since the driver is polled between every await, so the test says why it asks for eight threads. The default stays 0. Whether the pool can now be turned on is a question for the live lane, and it is the next step rather than part of this change. Closes #1758. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
#1734 quoted the seven `PodmanArgs=` interpolations in the container and build units and claimed the sweep was complete. It was not: the `.volume` unit routes an unrecognised `driver_opts` key through `PodmanArgs=` as well, and that site kept interpolating raw. An independent verification pass of the landed fixes found it, and I reproduced it: volumes: data: driver_opts: arbitrary: "value --label injected=yes" PodmanArgs=--opt arbitrary=value --label injected=yes These are `podman volume create` flags rather than `podman run` flags, so the blast radius is smaller than the original finding, but the mechanism is identical and the claim that every interpolated value was covered was false. The sweep stopped at the `.container` unit because that is where the reported input landed. The remaining three interpolations are `cpu_shares`, `cpu_quota` and `cpu_period`, typed `u64`/`i64`, so no string reaches them. That is now the whole set. An existing case asserted the unit text, `PodmanArgs=--opt custom=extra`, which the quoting changes. Measured against Podman 5.7.0's own generator, the quotes are consumed by systemd and it still builds `--opt custom=extra`, so the behaviour is unchanged and only the text moved. That assertion now reads the argv instead, for the reason #1734 gave for its own tests: the unit text is what looked correct for the whole life of the bug. Reverting the quoting turns the new case red on its own. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…lias guard (#1766) The second bypass of the same guard. Entering single-quote state did not require being outside double-quote state, so a `'` inside a double-quoted scalar set `in_single`, the closing `"` cleared only `in_double`, and the scanner stayed "inside a quoted scalar" for the rest of the document. Every `*alias` after that line was counted as text, `count_alias_refs` returned 0, the `refs == 0` early return fired, and both the reference cap and the document-size cap were skipped. Two files eighteen bytes apart, measured on the tree before this change: ctl.yaml (917 B) refused, 101 alias references exp.yaml (935 B) accepted, guards never consulted An independent verification measured the amplification at 1598 MB in 58 s against 21 MB in 0.01 s for the control. Both arms are symmetrical now: a quote character opens a scalar only when no other scalar is open. `#` already had it right, which is why the comment-not-a-comment hole was a different shape. The test's payload after the poisoning line carries no stray single quote, deliberately. The bypass is fragile with respect to state: a later `'` in value position closes it again and the document is refused normally, so a careless payload makes the case pass while the guard is working. A verification run's first attempt failed for exactly that reason. The control is asserted to be refused on its own, so a change that made both behave the same could not pass silently. Reverting only the single -quote arm turns the case red. This is the third hole in a hand-rolled scanner over YAML text, after the apostrophe in a plain scalar and the `#` that is not a comment. Counting `Event::Alias` from the loader would end the class rather than the instance, and `serde_yaml_ng` keeps that type crate-private, so it stays a scanner. That is worth revisiting if a fourth appears. Closes #1737. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…1767) The 4 MiB cap on a single interpolation pass works and bounds the wrong unit. `interpolate_value` calls substitution separately for each scalar with no shared budget, so repetition split across scalars is unbounded while every individual substitution stays legal. Measured before this change, with a 1 MiB `.env` value and one service whose environment reads it from N entries: 256 reached 1.6 GB, 512 reached 3.2 GB, and 1024 aborted at 5.27 GB. No single substitution exceeded 1 MiB, so the existing cap never fired once. One budget now spans the interpolation of a document, threaded from `interpolate_value` down to `substitute_depth` rather than kept in global state, which is what the fifteen updated test call sites cost. 16 MiB, picked by measurement. My first choice was 64 MiB and the measurement refused it: at that size a document of 64 one-mebibyte entries is accepted and costs 419 MB resident, which is not a bound worth having. At 16 MiB, eight entries still parse at 80 MB, thirty-two are refused in 1.3 s at 42 MB, and 256 in 1.2 s. It is also the number the per-file cap already uses, so a reader meets one size rather than two. The question that set the number was not "does it refuse the attack" but "what does it cost to accept what I still accept". The first answer was yes to both, which is how 64 MiB looked defensible in prose. The per-scalar cap stays as the cheaper first line: it refuses a single runaway value without walking the rest of the document. The refusal names the variable, so a large legitimate file is distinguishable from a hostile one. Making the comparison unreachable turns the new case red. Closes #1738. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Both were found by an independent verification of the landed code, not by a reviewer, and both were reproduced before being fixed. Directory pruning lost `.dockerignore` re-inclusion. With `vendor/` and `!vendor/keep.txt`, pruning `vendor` drops `keep.txt` from the context and a `COPY vendor/keep.txt` loses its source. The walk's own comment justified the shortcut by asserting the engine had "no negation patterns in the wild that would re-include a child of an ignored directory". That is an assertion about what users write, and the format supports negation so that they can. Pruning now happens only when no negation pattern could name something beneath the directory. The check is deliberately conservative: a false "could reach" costs one descent the leaf filter then discards, while a wrong "cannot" silently drops a file the user asked to keep. The optimisation still applies to the ordinary case it was written for, an ignored subtree with nothing re-included. The `extends` cache resolved every service in a referenced file before caching it, so referencing one valid service failed when an unrelated service in the same file extended something missing. A file you do not control could break your build over a service you never asked for. The cache now holds the parsed file and the caller resolves the chain it needs, which it was already doing on the next line. The parse is what the cache exists to save, and that saving is untouched: the parsed-at-most-once case stays green. The second test asserts both directions. Asking for the broken service still fails, so this does not simply stop reporting the failure. Reverting either fix turns its own case red and leaves the other green. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
…#1769) A builder stream chunk carrying several lines in one write became a single tail entry. The repaint arithmetic counts entries, so the board counted one row where the terminal drew seven, `cursor_up(painted)` landed six lines short from that frame on, and one build scrolled the same block down the terminal about eighty times. `Region`'s doc comment already stated the invariant: a line that wraps makes the terminal count two rows where this counted one, and from then on every repaint erases the wrong lines. The invariant was right and an embedded newline breaks it by a route the width fit does not cover, since `chars().take(width)` counts `\n` as an ordinary character. `note_lines` splits at ingestion, which is also where the trim and the drop-empty already lived, so one entry is one row again. `split('\n')` rather than `lines()` on purpose: `lines()` also swallows a trailing `\r`, and the `trim()` handles `\r\n` anyway, so the split and the trimming stay two decisions. It restores `MAX_NOTES_PER_ROW` as well. The cap counts entries, so one arbitrarily tall entry defeated it. `render_note` gains a `debug_assert!` that a note carries no newline, so the invariant is enforced where it is relied on rather than only where it is produced today. The split is a pure function with its own test hook, because the live path needs a region the cargo-test runner cannot give us and the existing hook enters below the split. Replacing it with a `std::iter::once` turns the multi-line case red on its own. Closes #1733. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
The entry was written when the bump was cut and named the two `audit` fixes that existed then. Thirty-two commits landed after it, eighteen of them fixes, including a privilege escalation: a compose file could put `--privileged --net=host -v /:/hostfs` into the generated Quadlet unit and neither the runtime warning nor `audit` said anything. A changelog that names two of eighteen is worse than a short one, because it reads as complete. The entry now leads with the five security items, since that is what decides whether somebody upgrades today, and lists the behaviour changes after them. Written for somebody running the tool: what stopped working, what started, what is now refused that was not. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com> Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
enabled auto-merge
September 8, 2026 02:02
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.
Merge commit, no tag in this pull request. Thirty-two commits past 5.9.1, eighteen of them fixes, five of those security.
The headline is a privilege escalation: a compose file could put
--privileged --net=host -v /:/hostfsinto the generated Quadlet unit throughPodmanArgs=, and neither the runtime warning norauditreported it. Closed in two passes, because the first sweep stopped at the.containerunit and left.volume.The rest:
autostart install --mode quadletskipping the validator,cpfollowing a symlink planted in the archive, two ways to switch off the YAML alias guards, and.dockerignorenever matching below the top level on Windows.All three files that stamp a version read 5.9.2, checked against
origin/developimmediately before this was opened. The tag goes onmainafter this merges, which is the only thing that triggers a release.Signed-off-by: Jaro-c 75870284+Jaro-c@users.noreply.github.com