Skip to content

Probe closed stdin with an fd duplication in _autolint_read_files0 - #128

Closed
cgraf78 wants to merge 4 commits into
mainfrom
auto/ci-fix-checkrun-20261103
Closed

cgraf78 wants to merge 4 commits into
mainfrom
auto/ci-fix-checkrun-20261103

Conversation

@cgraf78

@cgraf78 cgraf78 commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes the scheduled-CI failure in https://github.com/cgraf78/checkrun/actions/runs/41596810832 (test/suites/autolint-test: "files0 stdin: closed descriptor exits 2" and "closed descriptor is actionable").

Root cause (proven with two diagnostic runs on the PR branch, both Ubuntu and WSL legs): the ubuntu-26.04 runner's bash 5.3.9 reopens /dev/null for a closed stdin at process startup — a fresh bash -c probe started with closed stdin arrives with fd 0 -> /dev/null. So a subprocess invocation can never observe the closed descriptor the guard is meant to reject, and the old /dev/stdin readability probe passed while the read loop treated it as EOF. The reopened /dev/null is indistinguishable from legitimate < /dev/null input, so the tool must not treat /dev/null as closed.

Fix, in two parts:

  1. Tool (lib/checkrun/autolint.sh): replace the closed-detection half of the probe with a direct fd test — duplicating fd 0 into a subshell fails exactly when the descriptor is closed, independent of /dev symlink semantics. (Duplicating fd 0 onto itself is a no-op even when closed, so the target must differ.) The directory checks are unchanged.
  2. Test (test/suites/autolint-test, closed-stdin case only): invoke _autolint_main in-process via a command-substitution subshell instead of via bin/autolint. A subshell forks without re-running bash startup, so the closed fd survives to the guard. All other files0 tests still use bin/autolint and are unchanged.

Testing

  • Behavior matrix verified locally: closed stdin exits 2 with the actionable message (direct, via command substitution, and in the new in-process form, under both system bash and Ubuntu bash 5.3.9); empty pipe and /dev/null exit 0 quietly; directory stdin exits 2; a valid manifest reads normally; truncated input exits 2 with the NUL message.
  • bash -n and ShellCheck clean.
  • Full-suite verification via this PR's CI checks.

Review

Adversarial fresh-eyes review on correctness, edge cases, and tests/CI axes for the guard, plus a focused tests/CI review of the revised in-process test — all approved with no blocking findings.

Summary:
The `--files0-from -` guard rejected a closed descriptor by probing
`/dev/stdin` readability. On the ubuntu-26.04 runner that probe
misreports a closed stdin, so the read loop treated it as EOF and
the command exited 0 instead of 2 with the actionable message.
Replace the closed-detection half of the probe with a direct fd
test: duplicating fd 0 into a subshell fails exactly when the
descriptor is closed, independent of `/dev` symlink semantics.
(Duplicating fd 0 onto itself is a no-op even when closed, so the
target must differ.) The directory checks are unchanged.

Testing:
Behavior matrix verified locally: closed stdin exits 2 with the
actionable message (direct and via command substitution, as the
test invokes it); empty pipe and `/dev/null` exit 0 quietly;
directory stdin exits 2; a valid manifest reads normally;
truncated input exits 2 with the NUL message. `bash -n` and
ShellCheck are clean. Full-suite verification via this PR's CI
checks. Note: the dup probe is proven correct on every locally
testable fd state; the precise ubuntu-26.04 `/dev` mechanism behind
the original misreport remains a hypothesis.
Summary:
PR checks still fail the closed-stdin assertions with the fd-dup
probe, while every local replication rejects closed stdin. Add an
env-gated fd-state dump (CHECKRUN_DIAG_FD0, set only by the
closed-stdin test) so the runner's job log shows the real fd 0
state. Revert before merge.
Summary:
Follow-up diagnostic: run a fresh `bash -c` probe with closed stdin
in the same command-substitution form as the closed-stdin test, to
locate where fd 0 becomes /dev/null (bash startup vs the autolint
script path). Revert before merge.
Summary:
The runner's bash reopens /dev/null for a closed stdin at process
startup, so a subprocess invocation can never observe the closed
descriptor: replace the /dev/stdin readability probe with an
fd-duplication probe, and invoke the closed-stdin test in-process
(command-substitution subshell) where the closed fd survives.
Diagnostics from the investigation are reverted; the dup probe and
the in-process test are the whole change.
cgraf78 added a commit that referenced this pull request Nov 3, 2026
## Summary

Fixes the scheduled `Tests` failure in
<https://github.com/cgraf78/checkrun/actions/runs/41596810832>, where
`test/suites/autolint-test` failed `files0 stdin: closed descriptor
exits 2` and `... is actionable` on the Ubuntu and WSL legs after
`ubuntu-latest` moved to Ubuntu 26.04. Supersedes #128.

Root cause: the product guard was never at fault. The test closed fd 0
and then ran `bin/autolint` through its `#!/usr/bin/env bash` shebang.
Ubuntu 26.04's coreutils are uutils (the runner image lists `coreutils
9.5-1ubuntu2+0.0.0~ubuntu25`), so `/usr/bin/env` is a Rust binary, and
Rust's std runtime reopens any closed descriptor 0-2 on `/dev/null`
(O_RDWR) before `main`. autolint therefore received an open, empty
`/dev/null` stream, which is indistinguishable from a deliberate
`</dev/null` empty manifest, and exited 0 quietly.

- The closed-stdin case now runs as `"$BASH" "$AUTOLINT" --files0-from -
<&-`, so the closed descriptor reaches the guard on every platform. The
other `--files0-from -` cases still go through the shebang.
- `_autolint_read_files0` is unchanged: its `/dev/stdin` probe is
correct whenever fd 0 actually arrives closed. #128's fd-duplication
probe could not help, because by then fd 0 is genuinely open on
`/dev/null`; #128's later attribution to bash came from a probe that
itself ran through `/usr/bin/env bash -c`.
- No user-facing behavior changes. On uutils hosts, `autolint
--files0-from - <&-` launched through `PATH` still exits 0, since the
closed descriptor never reaches the process; nothing in-process can tell
that apart from `</dev/null`.

## Testing

- Reproduced locally with a minimal Rust exec wrapper standing in for
uutils `env`: fd 0 arrives as `lrwx ... -> /dev/null` and `autolint
--files0-from - <&-` exits 0 with no output (the CI signature); GNU
`env` and direct `bash` keep fd 0 closed and exit 2 with the actionable
message.
- #128's CI diagnostic showed fd 0 inside autolint as `lrwx`
`/dev/null`, matching Rust's O_RDWR reopen rather than a shell redirect.
- Mutation check: with the guard disabled, the updated invocation exits
0, so both assertions still catch a regression.
- `test/suites/autolint-test`: 517 passed, 0 failed.
- `test/checkrun-test`: everything passes except the `schema-refresh`
refs cases, which fail locally only because this host lacks the Python
`referencing` library; that suite is untouched and runs in CI.
- `checkrun format`, `checkrun lint`, and `shellcheck-inventory/run.sh`
at the locked cgraf78/actions SHA pass.

## Review

Fresh-eyes review by two independent reviewers, both approving:

- Root cause and correctness: independently confirmed the Rust std
O_RDWR reopen and that `"$BASH" "$AUTOLINT"` still exercises the real
guard, including when the suite itself is launched through a Rust `env`.
This PR's Ubuntu/WSL legs are the on-runner proof that bash 5.3 does not
reopen fd 0.
- Cross-platform side effects: `$BASH` is always the suite's own
absolute bash; BSD, GNU, and busybox `env` leave closed descriptors
alone; the WSL leg is simulated on the Ubuntu runner (same uutils
`env`); Termux does not run this suite; no docs promise closed-stdin
behavior. Addressed: commit header style and comment wording that now
attributes the reopen to Rust's std runtime.
@cgraf78

cgraf78 commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #129, which landed. Root cause was Ubuntu 26.04's uutils /usr/bin/env (Rust std reopens closed fds 0-2 on /dev/null), not bash; the product guard was correct and is unchanged.

@cgraf78 cgraf78 closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant