Skip to content

Commit 8158c4b

Browse files
senthilr-nvcv
andauthored
fix(e2e): honor managed gateway unit path (#9261)
<!-- markdownlint-disable MD041 --> ## Summary The protected Portable Profile `systemctl` fixture now reads the OpenShell gateway binary selected by the managed user service instead of assuming the user XDG bin directory. It accepts only the three installer-owned locations and rejects ambiguous or untrusted service identities. This restores the portable-launch fixture after automatic run `31969858915`, job `95220512323`, rejected a valid `/usr/local/bin/openshell-gateway` unit. The immutable artifact is `9269507500` with digest `sha256:2bc324b785af9d93abef8dc3a4bc4da38b4c2d30f51e7f88838e8cf0cd6315e0`. ## Related Issue Relates #9208. ## Changes - Read exactly one absolute `ExecStart` path from the managed OpenShell gateway user service. - Accept only the installer-supported XDG user bin, `/usr/local/bin`, or `/usr/bin` gateway locations. - Keep the exact `ExecStartPre`, unit marker, environment, file type, permission, and symlink checks fail closed. - Cover both system locations and an untrusted path with deterministic fixture tests. ## 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: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: This changes only the protected Portable Profile E2E fixture and its deterministic support coverage; production and supported user behavior are unchanged. - [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 nine-category security review of commit `6b8fc44b8` passed with no findings; the path allowlist matches installer authority and preserves existing unit, environment, process-identity, logging, and cleanup controls. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Independent Pi CLI review covered the complete two-file PR diff at latest PR commit `338dd2fd6`. The change is limited to an internal E2E fixture and support tests for installer-selected OpenShell gateway paths. It changes no public command, configuration, workflow, default, or supported behavior. The conflict-free main merge incorporates the terminal-test fix required by CI. - Agent: Pi CLI <!-- docs-review-head-sha: 338dd2f --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `./node_modules/.bin/vitest run --project e2e-support test/e2e/support/portable-profile-systemctl-shim.test.ts` passed 30/30 on exact current main. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this two-file fixture repair. Current-main `checks:repository`, CLI typecheck, source-shape, test-size, conditional, test-loop, and scoped normal hooks passed. - [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) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation of the managed gateway service configuration. * Gateway binaries must now use trusted system locations and be regular, readable executables. * Added checks to ensure service start commands consistently reference the configured gateway binary. * Improved error reporting when the gateway identity or executable configuration is invalid. * **Tests** * Added coverage for valid gateway paths in trusted locations. * Added validation for rejecting untrusted paths and missing executable output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
1 parent 2d03922 commit 8158c4b

2 files changed

Lines changed: 88 additions & 5 deletions

File tree

test/e2e/fixtures/portable-profile-systemctl-shim.sh

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ service_pid_file="${runtime_dir}/nemoclaw-podman-service.pid"
2020
log_file="${runtime_dir}/nemoclaw-podman-service.log"
2121
gateway_service_name="nemoclaw-openshell-gateway"
2222
gateway_unit_path="${config_home}/systemd/user/${gateway_service_name}.service"
23-
gateway_binary_path="${bin_home}/openshell-gateway"
23+
gateway_binary_path=""
2424
gateway_env_file="${config_home}/openshell/gateway.env"
2525
gateway_tls_dir="${state_home}/openshell/tls"
2626
gateway_state_dir="${state_home}/openshell/gateway"
@@ -460,6 +460,34 @@ stop_runtime() {
460460
rm -f "$socket_path" "$backend_socket_path"
461461
}
462462

463+
gateway_binary_path_is_trusted() {
464+
local candidate="$1"
465+
local user_bin_home="${bin_home%/}"
466+
case "$candidate" in
467+
"${user_bin_home}/openshell-gateway" | /usr/local/bin/openshell-gateway | /usr/bin/openshell-gateway)
468+
return 0
469+
;;
470+
*)
471+
return 1
472+
;;
473+
esac
474+
}
475+
476+
read_managed_gateway_binary_path() {
477+
local candidate="" count=0 line
478+
while IFS= read -r line || [[ -n "$line" ]]; do
479+
case "$line" in
480+
ExecStart=*)
481+
candidate="${line#ExecStart=}"
482+
((count += 1))
483+
;;
484+
esac
485+
done <"$gateway_unit_path"
486+
[[ "$count" -eq 1 && "$candidate" == /* && "$candidate" != *[[:space:]]* ]] || return 1
487+
gateway_binary_path_is_trusted "$candidate" || return 1
488+
printf '%s\n' "$candidate"
489+
}
490+
463491
validate_gateway_unit() {
464492
if [[ ! -f "$gateway_unit_path" || -L "$gateway_unit_path" || ! -r "$gateway_unit_path" ]]; then
465493
echo "Portable profile fixture requires the managed gateway user service at ${gateway_unit_path}." >&2
@@ -469,15 +497,21 @@ validate_gateway_unit() {
469497
echo "Portable profile fixture rejected the foreign gateway user service at ${gateway_unit_path}." >&2
470498
return 1
471499
fi
472-
if [[ "$(grep -Fxc "ExecStart=${gateway_binary_path}" "$gateway_unit_path" || true)" -ne 1 ]] \
473-
|| [[ "$(grep -Fxc "ExecStartPre=${gateway_binary_path} generate-certs --output-dir \${OPENSHELL_LOCAL_TLS_DIR} --server-san host.openshell.internal" "$gateway_unit_path" || true)" -ne 1 ]] \
500+
local unit_gateway_binary
501+
unit_gateway_binary="$(read_managed_gateway_binary_path)" || {
502+
echo "Portable profile fixture rejected the OpenShell gateway user service identity at ${gateway_unit_path}." >&2
503+
return 1
504+
}
505+
if [[ "$(grep -Fxc "ExecStart=${unit_gateway_binary}" "$gateway_unit_path" || true)" -ne 1 ]] \
506+
|| [[ "$(grep -Fxc "ExecStartPre=${unit_gateway_binary} generate-certs --output-dir \${OPENSHELL_LOCAL_TLS_DIR} --server-san host.openshell.internal" "$gateway_unit_path" || true)" -ne 1 ]] \
474507
|| [[ "$(grep -Fxc 'StateDirectory=openshell/gateway' "$gateway_unit_path" || true)" -ne 1 ]] \
475508
|| [[ "$(grep -Fxc 'Environment=OPENSHELL_LOCAL_TLS_DIR=%S/openshell/tls' "$gateway_unit_path" || true)" -ne 1 ]] \
476509
|| [[ "$(grep -Fxc 'EnvironmentFile=-%E/openshell/gateway.env' "$gateway_unit_path" || true)" -ne 1 ]] \
477-
|| [[ ! -x "$gateway_binary_path" || -L "$gateway_binary_path" ]]; then
478-
echo "Portable profile fixture rejected the gateway user service identity at ${gateway_unit_path}." >&2
510+
|| [[ ! -f "$unit_gateway_binary" || ! -r "$unit_gateway_binary" || ! -x "$unit_gateway_binary" || -L "$unit_gateway_binary" ]]; then
511+
echo "Portable profile fixture rejected the OpenShell gateway user service identity at ${gateway_unit_path}." >&2
479512
return 1
480513
fi
514+
gateway_binary_path="$unit_gateway_binary"
481515
}
482516

483517
load_gateway_environment() {

test/e2e/support/portable-profile-systemctl-shim.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,24 @@ function formatPsStartTime(scope: FixtureScope, startTime: string): ReturnType<t
241241
);
242242
}
243243

244+
function readManagedGatewayBinaryPath(scope: FixtureScope): ReturnType<typeof spawnSync> {
245+
return spawnSync(
246+
"bash",
247+
[
248+
"-c",
249+
'source "$1"\nread_managed_gateway_binary_path',
250+
"portable-profile-gateway-binary",
251+
scope.shim,
252+
],
253+
{
254+
encoding: "utf8",
255+
env: scope.env,
256+
killSignal: "SIGKILL",
257+
timeout: 15_000,
258+
},
259+
);
260+
}
261+
244262
function systemctlAsync(
245263
scope: FixtureScope,
246264
args: string[],
@@ -434,6 +452,37 @@ function portableLaunchStep(name: string): WorkflowStep {
434452
}
435453

436454
describe("portable profile systemctl fixture", () => {
455+
it.each(["/usr/local/bin/openshell-gateway", "/usr/bin/openshell-gateway"])(
456+
"reads installer-selected OpenShell gateway binary %s from the managed user service (#9208)",
457+
(gatewayBinary) => {
458+
const scope = createFixture();
459+
try {
460+
fs.writeFileSync(scope.gatewayUnitPath, gatewayServiceUnit(gatewayBinary), { mode: 0o600 });
461+
const result = readManagedGatewayBinaryPath(scope);
462+
expect(result.status, String(result.stderr)).toBe(0);
463+
expect(result.stdout).toBe(`${gatewayBinary}\n`);
464+
} finally {
465+
fs.rmSync(scope.directory, { force: true, recursive: true });
466+
}
467+
},
468+
);
469+
470+
it("rejects a managed OpenShell gateway user service that names an untrusted binary path (#9208)", () => {
471+
const scope = createFixture();
472+
try {
473+
fs.writeFileSync(
474+
scope.gatewayUnitPath,
475+
gatewayServiceUnit("/opt/openshell/bin/openshell-gateway"),
476+
{ mode: 0o600 },
477+
);
478+
const result = readManagedGatewayBinaryPath(scope);
479+
expect(result.status).not.toBe(0);
480+
expect(result.stdout).toBe("");
481+
} finally {
482+
fs.rmSync(scope.directory, { force: true, recursive: true });
483+
}
484+
});
485+
437486
it("normalizes irregular ps fallback spacing to one process-start-time identity (#9006)", () => {
438487
const scope = createFixture();
439488
try {

0 commit comments

Comments
 (0)