Skip to content

Commit 7f7126e

Browse files
jyaunchesgithub-actions[bot]
authored andcommitted
fix(e2e): remove unenforced PTY signing (#9421)
<!-- markdownlint-disable MD041 --> ## Summary Merged PR #9389 moved terminal-mode observation from reopening the root-owned PTY to the inherited descriptor, but its response-signing design staged a private key at a known path owned by the same sandbox UID. This follow-up removes the key writer, key file, and signing protocol while retaining inherited-descriptor observation, identity checks, and mode restrictions that limit unprivileged access to the sandbox UID. The E2E harness trusts that UID; authenticating a hostile same-UID process would require an upstream OpenShell boundary that attests the inherited process or provides direct execution. ## Related Issue Follow-up to #9384 and merged PR #9389. Original E2E root-cause key: `security-posture-openclaw::recorded-pty-termios-query`. Follow-up correction: remove the unenforced same-UID key-file boundary added while repairing that root cause. The post-merge runs below do not establish a complete pass for the affected OpenClaw live E2E targets: - [Run `32101568976`, job `95603202167`](https://github.com/NVIDIA/NemoClaw/actions/runs/32101568976/job/95603202167) failed the launch-readiness producer before the TUI verifier. - [Run `32101603265`, security-posture job `95604178951`](https://github.com/NVIDIA/NemoClaw/actions/runs/32101603265/job/95604178951) repeated the earlier launch-readiness publication failure. - [Run `32101603265`, gateway-guard job `95604178855`](https://github.com/NVIDIA/NemoClaw/actions/runs/32101603265/job/95604178855) failed the same publication gate after gateway recovery. - [Run `32101603265`, `full-e2e` job `95604179404`](https://github.com/NVIDIA/NemoClaw/actions/runs/32101603265/job/95604179404) reached the TUI but failed structured-turn evidence without the original `pty_termios_unavailable` signature. - [Run `32102697569`, job `95606379760`](https://github.com/NVIDIA/NemoClaw/actions/runs/32102697569/job/95606379760) passed the launch-readiness check and PTY qualification, then repeated the structured-turn failure after the first input. - [Run `32110926109`, job `95630372285`](https://github.com/NVIDIA/NemoClaw/actions/runs/32110926109/job/95630372285) tested candidate `6fd4d4574` before PR #9422 merged. Phases 1–3 passed. Phase 4 child lifecycle 16 passed the PTY input-mode gate, then failed with `launch did not record the required structured session turns`; cleanup passed. The log contains no `pty_termios_*` failure. This run does not evaluate `c7fdfe331`. - [Run `32112329966`, job `95634597620`](https://github.com/NVIDIA/NemoClaw/actions/runs/32112329966/job/95634597620) and [run `32112353122`, job `95636753522`](https://github.com/NVIDIA/NemoClaw/actions/runs/32112353122/job/95636753522) tested main commit `9ca0d6c98` after PR #9422 merged. Both passed the direct hosted-inference probe, in-sandbox inference request, gateway recovery, and launch-readiness probe. The first startup-aware launch then exited nonzero after about 13 seconds. The E2E test reported only `Error: launch session failed:` with no command diagnostic. The readiness failures have the distinct root key `security-posture-openclaw / launch-readiness producer publication / complete probe succeeds but final evidence cannot be verified or published`. Commit `b09eecef8` did not show this readiness failure; merged commit `7b43d6705` did. This evidence does not identify the introducing commit. The historical structured-turn root is `full-e2e / first structured PTY launch / no complete post-baseline user-assistant pair before the session deadline`. PR #9422, merged as `e11abd27a` and included in base commit `9ca0d6c98`, owns that correction. PR #9421 does not claim or duplicate it. The launch-readiness publication root remains outside PR #9421. The post-#9422 main failures have the separate root key `security-posture-openclaw / first startup-aware launch / nonzero exit with no captured command diagnostic`. PR #9421 does not claim to correct this failure. PR #9421's cleanup-diagnostic change preserves stderr from later cleanup actions, but it does not establish that cleanup caused these failures. Source inspection, a regression test, and live E2E evidence are required to establish any relationship. Exact-commit CI and two sequential `security-posture-openclaw` live E2E passes remain pending for `c7fdfe331`. Earlier runs do not satisfy those acceptance results. ## Changes - Remove Ed25519 generation, the extra OpenShell key-writer command, the sandbox key file, response signatures, and their cleanup lifecycle. - Return one fresh bounded termios observation per connection through the existing Unix socket. Keep the inherited PTY descriptor, exact run-directory, socket, and PTY identity checks, the absolute response deadline, bounded printable diagnostics, and fail-closed cleanup. - Retain readiness gating, bounded monitor shutdown, the unreadable-device-path regression, canonical-state rejection, malformed-response and PTY-identity rejection, two ordered turns, and monitor process-lifecycle coverage. - Report a residual monitor socket distinctly and verify that cleanup refuses to remove it. - Run every cleanup action before returning the original launch status, and remove host-side session state before the bounded PTY monitor wait. - Scale the deadlines for Linux PTY source tests through the repository `testTimeout` helper. - Preserve launch cleanup diagnostics and assert the structured-session baseline cleanup message. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent security review of the commit under review, `c7fdfe331`, passed all nine security categories under the explicit same-UID E2E harness trust boundary ([security-review comment](#9421 (comment))). - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence Not applicable; this PR does not change `scripts/prepare-dgx-station-host.sh`. - [ ] Tested on DGX Station - Tested commit: Not applicable. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable — `npm run validate:pr` passed on `c7fdfe331` after refreshing `origin/main`. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above - Fail-first at `9d6dd421c`: `npm exec -- vitest run --project e2e-support test/e2e/support/launch-agent-turn.test.ts -t 'intercepts one OpenClaw launch'` failed 2/2 because the merged harness made four OpenShell calls instead of three. - Correction at `9726c786c`: the same focused command passed 2/2. - At commit under review `c7fdfe331`, `npm exec -- vitest run --project e2e-support test/e2e/support/launch-agent-turn.test.ts`: 20 passed, 25 intentional Linux-only skips on macOS. - `npm exec -- vitest run --project integration test/growth-guardrails.test.ts`: 25 passed. - `git diff --check 9ca0d6c...c7fdfe3`: passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — exact-commit CI pending. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Documentation impact: none. This changes only the internal live E2E harness and does not change a supported command, configuration, workflow input, or operator action. --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent d52803f commit 7f7126e

2 files changed

Lines changed: 143 additions & 454 deletions

File tree

0 commit comments

Comments
 (0)