fix(certgen): stage temp dir inside output dir to fix cross-device rename - #1
Open
gracesmith6504 wants to merge 13 commits into
Open
fix(certgen): stage temp dir inside output dir to fix cross-device rename#1gracesmith6504 wants to merge 13 commits into
gracesmith6504 wants to merge 13 commits into
Conversation
…2210) * fix(tui): route warning logs to status bar instead of stderr tracing::warn/info/debug calls in the TUI crate write to stderr via the global tracing subscriber. In ratatui's alternate-screen/raw-mode, stderr writes corrupt the terminal layout. Error-state sandboxes amplify this as background gRPC polls fail every 2s tick. Replace all 25 tracing calls with app.status_text assignments for direct-access sites, and Vec<String> accumulation for the spawned start_port_forwards task. Add ForwardWarnings event variant to decouple forward warning delivery from the sandbox name in CreateResult, preventing downstream gRPC lookup failures. Closes NVIDIA#2120 Signed-off-by: Ian Miller <milleryan2003@gmail.com> * feat(tui): add ForwardWarnings event variant New event type for non-fatal port-forward warnings during sandbox creation. Keeps warning delivery separate from the sandbox name in CreateResult to avoid corrupting downstream gRPC lookups. Signed-off-by: Ian Miller <milleryan2003@gmail.com> * chore(tui): remove tracing dependency Compile-time guard against reintroducing stderr-writing tracing calls in the TUI crate. 14 other crates retain the dependency. Signed-off-by: Ian Miller <milleryan2003@gmail.com> --------- Signed-off-by: Ian Miller <milleryan2003@gmail.com>
Signed-off-by: Kirit93 <kthadaka@nvidia.com>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 8.3.1 to 8.3.2. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@f98e069...11f9893) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 8.3.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…or OpenShift overrides (NVIDIA#2125) Signed-off-by: ChristianZaccaria <christian.zaccaria.cz@gmail.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
NVIDIA#2070) * fix(core): pin supervisor image tag to gateway version for all drivers The Podman and Kubernetes drivers defaulted the supervisor image to `:latest` via DEFAULT_SUPERVISOR_IMAGE, while the Docker driver already resolved a version-pinned tag. Extract the tag resolution logic into openshell-core so all three drivers use the same OPENSHELL_IMAGE_TAG > IMAGE_TAG > CARGO_PKG_VERSION priority chain. Closes NVIDIA#2068 Signed-off-by: Florent Benoit <fbenoit@redhat.com> * refactor(core): simplify supervisor image tag resolver to slice-based API Remove the Docker driver's wrapper functions and call openshell_core::config::default_supervisor_image() directly. Simplify resolve_supervisor_image_tag to accept &[&str] instead of three separate parameters. Signed-off-by: Florent Benoit <fbenoit@nvidia.com> Signed-off-by: Florent Benoit <fbenoit@redhat.com> --------- Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Florent Benoit <fbenoit@nvidia.com>
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
* feat(kubernetes): add sidecar supervisor topology Add the Kubernetes sidecar supervisor topology, its Helm/Skaffold configuration, topology documentation, and sidecar e2e matrix coverage. Skip root-only sandbox identity rewriting when process enforcement is network-only so the low-permission sidecar process container can start successfully. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar process id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar process id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): avoid similar proxy id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * docs(kubernetes): clarify sidecar topology limits Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): keep sidecar process leaf capless Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): refresh sidecar provider env snapshots Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * test(supervisor): align hot-swap identity regression Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): stage sidecar mtls files before proxy chown Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): simplify sidecar supervisor topology Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * chore(helm): reuse sidecar skaffold values Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar iptables helper names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(e2e): harden kube gateway wrapper setup Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid nft batch rollback on OCP Run nftables setup as individual commands so optional conntrack and log expressions can fail without rolling back required table, chain, and reject rules. Signed-off-by: Seth Jennings <sjenning@redhat.com> Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): preserve process identity in sidecar topology Render sidecar pods with a shared process namespace, keep binary-aware network policy enabled, and move Kubernetes sidecar settings under the nested sidecar config table. Also apply unprivileged Landlock/seccomp setup in NetworkOnly supervisor mode so sidecar topology keeps sandbox child hardening without privileged process setup. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * refactor(kubernetes): replace sidecar snapshots with control socket Coordinate sidecar policy and provider bootstrap over a local Unix socket so the process leaf no longer reads policy/provider snapshot files. Report entrypoint startup through the control channel and keep gateway credentials confined to the network sidecar. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * feat(kubernetes): support relaxed sidecar network identity Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): satisfy sidecar clippy lint Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * refactor(kubernetes): standardize topology naming Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): satisfy linux clippy timeout import Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): support kata sidecar on ipv4 pods Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): satisfy linux clippy for sidecar fallback Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * chore(kubernetes): remove stale supervisor topology references Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): enable sidecar binary policy inspection Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): harden sidecar control boundary Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): couple sidecar supervisor lifecycles Signed-off-by: Taylor Mutch <taylormutch@gmail.com> --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com> Signed-off-by: Seth Jennings <sjenning@redhat.com> Co-authored-by: Seth Jennings <sjenning@redhat.com>
* feat(kubernetes): support PVC subPath driver config Signed-off-by: mjamiv <michael.commack@gmail.com> * test(kubernetes): cover writable PVC driver config Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC subPath review feedback Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC config review follow-up Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC review follow-ups --------- Signed-off-by: mjamiv <michael.commack@gmail.com>
…itten (NVIDIA#2162) * fix(network): fail closed when credential placeholders cannot be rewritten When the credential rewriter degrades internally, the proxy forwarded the literal `openshell:resolve:env:<NAME>` placeholder (or its provider alias marker) to the upstream instead of the resolved secret, leaking the reserved token on the wire and causing upstream auth failures (NVIDIA#2161). Two fail-open paths are closed: - secrets: `rewrite_http_header_block` returned the header block verbatim when no `SecretResolver` was available, so the fail-closed marker scan (which ran only on the resolved path) never saw the placeholder. It now scans the header region for reserved markers even with no resolver and returns `UnresolvedPlaceholderError` when one is present. Marker-free traffic still passes through unchanged. - proxy: when TLS was detected on a CONNECT but `tls_state` was `None` (ephemeral CA generation or CA file write failed at startup), the handler fell back to a raw `copy_bidirectional` tunnel, bypassing credential rewrite. Inside the proxy handler `tls_state` is `None` only on CA-init failure (`mode != Proxy` never starts the handler, and `tls: skip` is handled earlier), so it now refuses the connection with a 503 and a High-severity denial event instead of tunneling. The two startup CA-failure logs are raised from Medium to High. Tests: resolver=None with a placeholder in the request line, a header value, and the provider-alias form now fail closed; marker-free passthrough is unchanged; the relay integration test asserts the request is rejected before any byte reaches upstream; and the 503 fail-closed response contract is locked. Signed-off-by: Tony Luo <xialuo@nvidia.com> * fix(network): refuse CONNECT before 200 when TLS termination is unavailable The fail-closed refusal for a terminating CONNECT with no TLS termination state (ephemeral CA init failed) was written after the 200 Connection Established response. Because a CONNECT client only sends its TLS ClientHello after reading the 200, the peek-based TLS detection is inherently post-200, so the 503 landed inside the established tunnel and surfaced to the client as a TLS protocol error rather than a readable status. An 'allowed CONNECT' event was also logged first. Move the decision to a pre-200 gate: query_tls_mode resolves purely from the policy decision + host/port (no peeked bytes), so the route's TLS treatment is known before the tunnel is acknowledged. When TLS state is absent and the route is not tls: skip, write the 503 as the first bytes on the socket, emit the High-severity Denied event, and close. tls: skip routes tunnel raw exactly as before, and no allowed-CONNECT event is emitted on the refusal path. The now-unreachable post-200 branch is kept as defense in depth but no longer writes an in-tunnel 503; it fails closed by dropping the connection instead. Add connection-level regression tests over a real loopback socket: the gate refuses with HTTP/1.1 503 as the first bytes for a terminating route, and writes nothing when TLS termination is present or the route is tls: skip. Signed-off-by: Tony Luo <xialuo@nvidia.com> * fix(network): order the CONNECT TLS-unavailable refusal after SSRF Addresses the gator re-check on NVIDIA#2162. Ordering: the pre-200 fail-closed refusal ran before SSRF/allowed_ips validation, so during CA-init failure an internal-address CONNECT got a 503 tls_termination_unavailable instead of the normal 403 ssrf_denied, weakening operator visibility in degraded state. The SSRF branches now return validated addresses; the refusal runs after that validation (an internal address has already been denied with 403) but still before the upstream connect and before 200 Connection Established. effective_tls_skip is still resolved up front since the refusal consumes it. Tests: add connection-level regressions through the real handle_tcp_connection, driving a CONNECT from a child /bin/bash copy so the /proc process-identity binding resolves it against a permissive policy (the hot-swap test's identity pattern). They assert the first bytes are HTTP/1.1 503 for a terminating route with no TLS state, a 403 (not 503) for an internal address, and no refusal for a tls: skip route. These are gated to Linux at runtime (evaluate_opa_tcp needs /proc); a companion test verifies the OPA policy shape (glob allow, tls mode) on every platform so the precondition is locked where /proc is unavailable. rest.rs: tighten the fail-closed relay test to assert the forwarded buffer is_empty() rather than merely lacking the placeholder/secret. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(network): keep the CONNECT handler test client fork-free The handler regression tests forked cat to read the proxy reply, so the client socket fd was inherited by a second process with a different binary. The identity resolver correctly denies that as ambiguous shared-socket ownership (the same invariant resolve_process_identity_denies_fork_exec_shared_socket_ambiguity pins), so the tests exercised the deny path instead of the allow path — and on busy CI runners the deny-path /proc fallback scan exceeded the test budget and looked like a hang. The client script now uses only bash builtins (exec, printf, read -d '') so exactly one process owns the socket, and the child is left to exit on EOF instead of being killed mid-read. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(network): drive the CONNECT handler tests with an in-process client The child-process client (even fork-free) made the handler tests environment-sensitive: on CI runners with a busy or restricted /proc, resolving the child's socket ownership degraded into the whole-/proc fallback scan and a deny, which surfaced as a hang. The client is now an in-process TcpStream and the test policy allows current_exe(), so identity resolution binds the socket to the test process itself in the descendant scan — the same in-process pattern the passing resolve_process_identity tests rely on. The tls: skip test additionally asserts that the handler emitted no DenialEvent at any stage, so it can no longer pass vacuously on a policy or identity deny. Refusal budgets widened to 30s as a belt for slow runners; the refusals themselves return in milliseconds. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(core): pin the percent-encoded marker no-resolver fail-closed path The no-resolver scan already catches the percent-encoded canonical marker through its decoded pass; this regression pins it: a request line carrying openshell%3Aresolve%3Aenv%3AKEY with no resolver must fail closed with UnresolvedPlaceholderError { location: header }. Signed-off-by: Tony Luo <xialuo@nvidia.com> --------- Signed-off-by: Tony Luo <xialuo@nvidia.com>
gracesmith6504
force-pushed
the
fix/2173-certgen-cross-device-rename
branch
from
July 12, 2026 20:21
ea0967a to
579126e
Compare
|
I have build this on my machine and it works. |
…name generate-certs stages temp files beside --output-dir using dir.with_file_name(). When --output-dir is a container or WSL volume mount, the staging dir lands on a different filesystem and std::fs::rename fails with EXDEV. Move staging inside the output dir so the rename always stays on the same filesystem, preserving atomic replacement. Fixes NVIDIA#2173 Signed-off-by: Grace Smith <grasmith@redhat.com>
gracesmith6504
force-pushed
the
fix/2173-certgen-cross-device-rename
branch
from
July 13, 2026 09:48
579126e to
d7ffd0a
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. |
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
11 participants
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.
Summary
The temp dir was placed beside the output dir instead of inside it. When
--output-diris a volume mount, the sibling ends up on a different filesystem --std::fs::renamefails with EXDEV (os error 18). Moving the temp inside the output dir keeps both on the same filesystem.Related Issue
NVIDIA#2173
Changes
staging_temp_dir: usedir.join(".certgen.tmp")instead ofdir.with_file_name(...)-- places temp inside the output dirwrite_local_bundle,write_local_tls_bundle,write_local_jwt_bundle: movecreate_dir_restrictedcalls beforestaging_temp_dirso the output dir exists before staging inside itsibling_temp_dirtostaging_temp_dirto reflect the new placementTesting
mise run pre-commitpassesReproduced EXDEV on WSL with Podman + named volume, confirmed patched binary succeeds with "PKI files created dir=/extvol".
Checklist