credential: address the four remaining findings from the PR #127 review

[congwang-mk]

Follow-up to #127, addressing the four findings that remained after the re-review:

1. Seekable stdin rejected for fd:0. Stdin survives into the child (unlike fds above 2, which are closed post-fork), and a dup shares the file offset, so a < secret.txt redirect would let the child lseek back and re-read the secret even after the supervisor drained it. load_secret now refuses a rewindable fd 0 and points at the pipe pattern; pipes/sockets/ttys (ESPIPE) stay allowed, and seekable sources on fds above 2 are unaffected.

2. Header names validated at rule-parse time. A malformed header:<name> / apikey:<name> previously surfaced only as a per-request 502 the first time the rule fired; parse_auth now runs HeaderName::from_bytes so a typo fails the build like every other spec error.

3. File-overlap warning honors fs-deny. The warning suggested "add an fs-deny for it" but did not consult fs_denied, so following the advice still warned on every run. The overlap check (extracted into a testable exposing_grant helper) now suppresses the warning when a deny covers the secret file.

4. Query param names compared percent-decoded. A child spelling the target param with stray percent-encoding (%6Bey= for key=) evaded the Replace de-dup filter, leaving its pair alongside the injected one. Param names are now decoded before comparison, for both the Replace filter and the AddOnly presence check.

Each fix comes with unit tests, including a stdin-swap test that exercises both the file-redirect rejection and the pipe acceptance on the real fd 0.

🤖 Generated with Claude Code

[congwang-mk]

@dzerik Please take a look. Thanks!

[dzerik]

Reviewed the four follow-ups against the code and traced enforcement on both the notif and chroot paths. Three are solid and complete:

• Seekable fd:0: the stream_position() probe rejects a < file redirect (lseek succeeds) while pipes/sockets/ttys stay allowed (ESPIPE), and fds >2 are unaffected since they're closed in the child. Right correction to the earlier "allow fd:0 unconditionally".
• Header validation at parse time: HeaderName::from_bytes in parse_auth moves a bad header:/apikey: name from a per-request 502 to a build error.
• Percent-decoded query de-dup: is_target decodes for both the Replace filter and the AddOnly check; percent_decode_lossy is bounds-safe and case-insensitive, so %6Bey= no longer evades. (Only residual: +→space form-encoding and non-conformant server query parsers — a one-line doc note that query:<param> names should be token-safe covers it, not a blocker.)

One residual on the fs-deny suppression (fix 3), chroot-only. For non-chroot grants it's correct — fs_denied is enforced with inode identity via DeniedSet, so a host-path deny matches even through a bind mount, and the host-path suppression in exposing_grant aligns. But under chroot the namespaces diverge: exposing_grant decides suppression on the canonicalized host path, while chroot_denied = fs_denied.clone() is matched by ChrootCtx::is_denied as a virtual (in-jail) prefix, with no inode fallback. With root /jail and file:/jail/etc/token, adding --fs-deny /jail/etc/token (the host path the warning points at) silences the warning — but at runtime the child opens /etc/token, "/etc/token".starts_with("/jail/etc/token") is false, and the read succeeds. So the operator is told the hole is closed when it isn't; denying the correct virtual path /etc/token enforces but never silences, so the warning fires every run.

Suggest: when chroot is set, translate the secret's host path into the virtual namespace before the covered_by deny test (and advise the in-jail path in the warning), plus a chroot-mode test alongside exposing_grant_reports_overlap_and_fs_deny_suppresses. Since it only affects chroot + an in-jail file: secret and degrades a warning rather than opening an exploit outside chroot, I'm fine landing this and fixing the chroot suppression as a follow-up. Approve-with-notes.