Skip to content

Commit 6fc834c

Browse files
authored
fix(hermes): read API port from gateway identity (#9059)
<!-- markdownlint-disable MD041 --> ## Summary Hermes MCP transactions now resolve the sandbox API port from the validated same-identity gateway process. They no longer read the protected service-manager environment, which OpenShell denies across identities. ## Related Issue Fixes #9044 ## Changes - Read `NEMOCLAW_HERMES_API_PORT` from `/proc/<gateway-pid>/environ` after the existing gateway PID, owner, launcher, managed-parent, and start-time checks. - Recheck the gateway identity after the bounded environment read. Reject an unavailable, oversized, malformed, ambiguous, or identity-changed source without logging environment values. - Add a real same-identity process regression test that fails when the helper reads the service-manager PID. Keep focused negative tests for access denial, invalid ports, duplicate values, and identity changes. - Update the OpenShell 0.0.101 child-environment manifest with the changed helper's exact SHA-256 integrity value and keep its migration-review expectation aligned. - Root cause: the per-sandbox port change selected the service manager as the environment source. Existing tests mocked that cross-identity read and did not exercise the Linux process boundary. ## 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: The API-port range, default, onboarding input, MCP commands, and lifecycle behavior do not change. - [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: The nine-category review passed with no findings for commit `197b2c4160`: #9059 (comment) - [ ] 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: The change corrects an internal Hermes MCP API-port lookup. Existing documentation remains accurate for the environment-variable range, default, onboarding input, MCP commands, and lifecycle behavior. The reviewer found no blocker or writing suggestion. - Agent: Codex Desktop <!-- docs-review-head-sha: 197b2c4 --> <!-- 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 — `npx vitest run --project integration test/hermes-mcp-api-port.test.ts test/hermes-mcp-probe-api-port.test.ts test/hermes-mcp-config-transaction.test.ts test/hermes-mcp-apply-race.test.ts test/hermes-mcp-rollback-pending.test.ts test/hermes-mcp-integrity-state.test.ts test/hermes-mcp-reload-convergence.test.ts test/hermes-mcp-force-cleanup.test.ts test/hermes-mcp-private-target-validation.test.ts test/openshell-0.0.101-migration-review.test.ts`: 10 files and 73 tests passed after the integrity-pin repair. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run; this change is limited to one Hermes transaction helper and its focused integration tests. - [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: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved gateway API port resolution by reading environment settings directly from the gateway process. * Added clearer handling when gateway environment information is unavailable or the process identity changes. * Improved validation of gateway process environment access for more reliable configuration behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent 0497b56 commit 6fc834c

4 files changed

Lines changed: 117 additions & 52 deletions

File tree

agents/hermes/mcp-config-transaction.py

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
MAX_GATEWAY_PID_RECORD_BYTES = 4096
9696
MCP_RACE_RECOVERY_ATTEMPTS = 3
9797
MAX_GATEWAY_PUBLIC_PORT_RECORD_BYTES = 16
98-
MAX_SERVICE_MANAGER_ENVIRONMENT_BYTES = 64 * 1024
98+
MAX_GATEWAY_ENVIRONMENT_BYTES = 64 * 1024
9999
GATEWAY_INTERNAL_PORT = 18642
100100

101101

@@ -1040,36 +1040,24 @@ def _gateway_identity() -> tuple[int, object] | None:
10401040
return numeric_pid, start_time
10411041

10421042

1043-
def _read_service_manager_environment(pid: int) -> bytes:
1043+
def _read_gateway_environment(pid: int) -> bytes:
10441044
try:
10451045
with open(f"/proc/{pid}/environ", "rb") as environment_file:
1046-
raw = environment_file.read(MAX_SERVICE_MANAGER_ENVIRONMENT_BYTES + 1)
1047-
except FileNotFoundError as error:
1048-
raise PermissionError(
1049-
"Hermes service-manager environment is unavailable"
1050-
) from error
1051-
if len(raw) > MAX_SERVICE_MANAGER_ENVIRONMENT_BYTES:
1052-
raise PermissionError("Hermes service-manager environment is too large")
1046+
raw = environment_file.read(MAX_GATEWAY_ENVIRONMENT_BYTES + 1)
1047+
except OSError as error:
1048+
raise PermissionError("Hermes gateway environment is unavailable") from error
1049+
if len(raw) > MAX_GATEWAY_ENVIRONMENT_BYTES:
1050+
raise PermissionError("Hermes gateway environment is too large")
10531051
return raw
10541052

10551053

1056-
def _service_manager_gateway_public_port(
1054+
def _gateway_environment_public_port(
10571055
identity: tuple[int, object],
10581056
) -> int:
10591057
gateway_pid = identity[0]
1060-
manager_pid = _process_parent_pid(gateway_pid)
1061-
if manager_pid is None or not _is_service_manager_process(manager_pid):
1062-
raise PermissionError(
1063-
"Hermes gateway is not running under the managed service lifecycle"
1064-
)
1065-
1066-
environment = _read_service_manager_environment(manager_pid)
1067-
if (
1068-
_gateway_identity() != identity
1069-
or _process_parent_pid(gateway_pid) != manager_pid
1070-
or not _is_service_manager_process(manager_pid)
1071-
):
1072-
raise PermissionError("Hermes service-manager identity changed while reading")
1058+
environment = _read_gateway_environment(gateway_pid)
1059+
if _gateway_identity() != identity:
1060+
raise PermissionError("Hermes gateway identity changed while reading")
10731061

10741062
prefix = b"NEMOCLAW_HERMES_API_PORT="
10751063
values = [
@@ -1078,15 +1066,13 @@ def _service_manager_gateway_public_port(
10781066
if entry.startswith(prefix)
10791067
]
10801068
if len(values) > 1:
1081-
raise PermissionError("Hermes service-manager API port is ambiguous")
1069+
raise PermissionError("Hermes gateway API port is ambiguous")
10821070
if not values or not values[0]:
10831071
return 8642
10841072
try:
10851073
decoded = values[0].decode("ascii")
10861074
except UnicodeDecodeError as error:
1087-
raise PermissionError(
1088-
"Hermes service-manager API port is malformed"
1089-
) from error
1075+
raise PermissionError("Hermes gateway API port is malformed") from error
10901076
return _parse_gateway_public_port(decoded)
10911077

10921078

@@ -1099,7 +1085,7 @@ def _resolve_gateway_public_port() -> int:
10991085
identity = _gateway_identity()
11001086
if identity is None:
11011087
raise PermissionError("Hermes gateway identity is unavailable")
1102-
return _service_manager_gateway_public_port(identity)
1088+
return _gateway_environment_public_port(identity)
11031089

11041090

11051091
def _configure_gateway_public_port() -> None:

src/lib/actions/sandbox/openshell-child-visible-credentials.v0.0.101.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
},
4242
{
4343
"path": "agents/hermes/mcp-config-transaction.py",
44-
"sha256": "01096bf959d07493ada4525ae824f3fabcde02fa6c02b8b56c16f227489dae0a"
44+
"sha256": "e3e51798c242b7ed54c1dff8203d3e73dbc2b9fcb8c7d271292f6b41f08bdd90"
4545
}
4646
]
4747
},

test/hermes-mcp-api-port.test.ts

Lines changed: 101 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import { spawnSync } from "node:child_process";
4+
import { spawn, spawnSync } from "node:child_process";
55
import path from "node:path";
66
import { describe, expect, it } from "vitest";
77

@@ -12,7 +12,43 @@ const TRANSACTION = path.resolve(
1212
);
1313

1414
describe("Hermes MCP API port resolution", () => {
15-
it("accepts only allocated ports from the stable service-manager environment (#8543)", () => {
15+
it("reads the port from a same-identity gateway process environment (#9044)", () => {
16+
const gateway = spawn(process.execPath, ["-e", "setTimeout(() => {}, 10000)"], {
17+
env: { NEMOCLAW_HERMES_API_PORT: "8645", PATH: process.env.PATH },
18+
stdio: "ignore",
19+
});
20+
21+
try {
22+
expect(gateway.pid).toBeTypeOf("number");
23+
const result = spawnSync(
24+
"python3",
25+
[
26+
"-c",
27+
`
28+
import importlib.util, json, sys, types
29+
sys.modules["yaml"] = types.SimpleNamespace(YAMLError=type("YAMLError", (Exception,), {}))
30+
spec = importlib.util.spec_from_file_location("mcp_tx", sys.argv[1])
31+
module = importlib.util.module_from_spec(spec)
32+
sys.modules[spec.name] = module
33+
spec.loader.exec_module(module)
34+
identity = (int(sys.argv[2]), 333)
35+
module._gateway_identity = lambda: identity
36+
print(json.dumps({"port": module._gateway_environment_public_port(identity)}))
37+
`,
38+
TRANSACTION,
39+
String(gateway.pid),
40+
],
41+
{ encoding: "utf8" },
42+
);
43+
44+
expect(result.status, result.stderr).toBe(0);
45+
expect(JSON.parse(result.stdout)).toEqual({ port: 8645 });
46+
} finally {
47+
gateway.kill("SIGKILL");
48+
}
49+
});
50+
51+
it("reads allocated ports from the identity-bound gateway environment (#9044)", () => {
1652
const result = spawnSync(
1753
"python3",
1854
[
@@ -26,17 +62,15 @@ sys.modules[spec.name] = module
2662
spec.loader.exec_module(module)
2763
identity = (41, 333)
2864
module._gateway_identity = lambda: identity
29-
module._process_parent_pid = lambda pid: 40
30-
module._is_service_manager_process = lambda pid: True
65+
opened = []
3166
3267
accepted = []
3368
for raw in (b"8642", b"8645", b"8652"):
34-
module._read_service_manager_environment = (
35-
lambda pid, value=raw: b"PATH=/usr/bin\\0NEMOCLAW_HERMES_API_PORT="
36-
+ value
37-
+ b"\\0"
69+
module._read_gateway_environment = (
70+
lambda pid, value=raw: opened.append(pid)
71+
or b"PATH=/usr/bin\\0NEMOCLAW_HERMES_API_PORT=" + value + b"\\0"
3872
)
39-
accepted.append(module._service_manager_gateway_public_port(identity))
73+
accepted.append(module._gateway_environment_public_port(identity))
4074
4175
rejected = []
4276
for raw in (
@@ -45,24 +79,25 @@ for raw in (
4579
"²".encode("utf-8"),
4680
b"8645\\0NEMOCLAW_HERMES_API_PORT=8646",
4781
):
48-
module._read_service_manager_environment = (
49-
lambda pid, value=raw: b"NEMOCLAW_HERMES_API_PORT=" + value + b"\\0"
82+
module._read_gateway_environment = (
83+
lambda pid, value=raw: opened.append(pid)
84+
or b"NEMOCLAW_HERMES_API_PORT=" + value + b"\\0"
5085
)
5186
try:
52-
module._service_manager_gateway_public_port(identity)
87+
module._gateway_environment_public_port(identity)
5388
except PermissionError as error:
5489
rejected.append(str(error))
5590
56-
module._read_service_manager_environment = lambda pid: b"PATH=/usr/bin\\0"
57-
absent = module._service_manager_gateway_public_port(identity)
91+
module._read_gateway_environment = lambda pid: opened.append(pid) or b"PATH=/usr/bin\\0"
92+
absent = module._gateway_environment_public_port(identity)
5893
5994
module._gateway_identity = lambda: (41, 999)
60-
module._read_service_manager_environment = (
61-
lambda pid: b"NEMOCLAW_HERMES_API_PORT=8645\\0"
95+
module._read_gateway_environment = (
96+
lambda pid: opened.append(pid) or b"NEMOCLAW_HERMES_API_PORT=8645\\0"
6297
)
6398
identity_change = ""
6499
try:
65-
module._service_manager_gateway_public_port(identity)
100+
module._gateway_environment_public_port(identity)
66101
except PermissionError as error:
67102
identity_change = str(error)
68103
@@ -71,6 +106,7 @@ print(json.dumps({
71106
"rejected": rejected,
72107
"absent": absent,
73108
"identity_change": identity_change,
109+
"opened": opened,
74110
}))
75111
`,
76112
TRANSACTION,
@@ -84,11 +120,54 @@ print(json.dumps({
84120
rejected: [
85121
"Hermes API port is outside the allocated range",
86122
"Hermes API port is outside the allocated range",
87-
"Hermes service-manager API port is malformed",
88-
"Hermes service-manager API port is ambiguous",
123+
"Hermes gateway API port is malformed",
124+
"Hermes gateway API port is ambiguous",
89125
],
90126
absent: 8642,
91-
identity_change: "Hermes service-manager identity changed while reading",
127+
identity_change: "Hermes gateway identity changed while reading",
128+
opened: Array(9).fill(41),
129+
});
130+
});
131+
132+
it("rejects an unavailable gateway environment without using another identity (#9044)", () => {
133+
const result = spawnSync(
134+
"python3",
135+
[
136+
"-c",
137+
`
138+
import builtins, importlib.util, json, sys, types
139+
sys.modules["yaml"] = types.SimpleNamespace(YAMLError=type("YAMLError", (Exception,), {}))
140+
spec = importlib.util.spec_from_file_location("mcp_tx", sys.argv[1])
141+
module = importlib.util.module_from_spec(spec)
142+
sys.modules[spec.name] = module
143+
spec.loader.exec_module(module)
144+
145+
opened = []
146+
real_open = builtins.open
147+
def denied(path, *args, **kwargs):
148+
opened.append(path)
149+
raise PermissionError("denied")
150+
151+
builtins.open = denied
152+
message = ""
153+
try:
154+
module._read_gateway_environment(41)
155+
except PermissionError as error:
156+
message = str(error)
157+
finally:
158+
builtins.open = real_open
159+
160+
print(json.dumps({"message": message, "opened": opened}))
161+
`,
162+
TRANSACTION,
163+
],
164+
{ encoding: "utf8" },
165+
);
166+
167+
expect(result.status, result.stderr).toBe(0);
168+
expect(JSON.parse(result.stdout)).toEqual({
169+
message: "Hermes gateway environment is unavailable",
170+
opened: ["/proc/41/environ"],
92171
});
93172
});
94173

@@ -227,7 +306,7 @@ print(json.dumps({
227306
});
228307
});
229308

230-
it("prefers the marker over the service-manager environment (#8543)", () => {
309+
it("prefers the root marker over the gateway environment (#8543)", () => {
231310
const result = spawnSync(
232311
"python3",
233312
[
@@ -241,7 +320,7 @@ sys.modules[spec.name] = module
241320
spec.loader.exec_module(module)
242321
243322
module._gateway_identity = lambda: (41, 333)
244-
module._service_manager_gateway_public_port = lambda identity: 8649
323+
module._gateway_environment_public_port = lambda identity: 8649
245324
246325
module._root_gateway_public_port_marker = lambda: 8647
247326
marker_wins = module._resolve_gateway_public_port()

test/openshell-0.0.101-migration-review.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ describe("OpenShell 0.0.101 migration review", () => {
230230
},
231231
{
232232
path: "agents/hermes/mcp-config-transaction.py",
233-
sha256: "01096bf959d07493ada4525ae824f3fabcde02fa6c02b8b56c16f227489dae0a",
233+
sha256: "e3e51798c242b7ed54c1dff8203d3e73dbc2b9fcb8c7d271292f6b41f08bdd90",
234234
},
235235
],
236236
});

0 commit comments

Comments
 (0)