Skip to content

Commit 59e05f0

Browse files
authored
Merge branch 'main' into fix/10085-config-get-doc-example
2 parents a2304d7 + 7e7c814 commit 59e05f0

25 files changed

Lines changed: 631 additions & 208 deletions

.github/workflows/pr-review-advisor.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,9 @@ jobs:
241241
# analysis-phase step with a GitHub token, and it has no model credential.
242242
- name: Prepare advisor sandbox inputs
243243
env:
244+
BASE_REF: ${{ github.event_name == 'pull_request_target' && 'target/base' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'target/base' || inputs.base_ref) }}
244245
GH_TOKEN: ${{ github.token }}
246+
HEAD_REF: ${{ github.event_name == 'pull_request_target' && 'HEAD' || (github.event_name == 'workflow_dispatch' && inputs.target_repo != '' && inputs.target_pr != '' && 'HEAD' || inputs.head_ref) }}
245247
run: node --experimental-strip-types --no-warnings "$ADVISOR_DIR/tools/pr-review-advisor/openshell.mts" prepare
246248

247249
- name: Install OpenShell

ci/cli-test-timing-hints.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
"src/lib/state/portable-uninstall-retirement.test.ts": 27072,
5959
"src/lib/tunnel/services.test.ts": 5573,
6060
"test/brev-launchable-e2e.test.ts": 34518,
61-
"test/channels-remove-full-teardown.test.ts": 9727,
61+
"test/channels/channels-remove-full-teardown.test.ts": 9727,
6262
"test/cli-oclif-compatibility.test.ts": 7416,
6363
"test/cli/connect-recovery.test.ts": 14143,
6464
"test/cli/credentials-command.test.ts": 6511,
@@ -81,7 +81,7 @@
8181
"test/cli/sandbox-status-text.test.ts": 14597,
8282
"test/cli/snapshot-shields.test.ts": 14050,
8383
"test/cli/tunnel-command.test.ts": 16882,
84-
"test/credentials.test.ts": 6328,
84+
"test/credentials/credentials.test.ts": 6328,
8585
"test/dcode-session-supervisor.test.ts": 7644,
8686
"test/dcode-wrapper-identity.test.ts": 11176,
8787
"test/deepagents-code-tui-startup-check.test.ts": 28222,

ci/source-shape-test-budget.json

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,6 @@
9696
"test": "selects exact-commit rootless evidence for Portable recovery changes (#9707)",
9797
"category": "security"
9898
},
99-
{
100-
"file": "test/growth-guardrails-workflow-boundary.test.ts",
101-
"test": "runs the trusted Vitest guardrails against pull request data",
102-
"category": "security"
103-
},
10499
{
105100
"file": "test/hermes-runtime-config-guard-topology.test.ts",
106101
"test": "allows the sandbox identity to create runtime state but refuses sealed configuration writes, unlinks, and renames (#7865)",
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { readFileSync } from "node:fs";
5+
import { dirname, join } from "node:path";
6+
import { fileURLToPath } from "node:url";
7+
import { isDeepStrictEqual } from "node:util";
8+
9+
import YAML from "yaml";
10+
11+
const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", "..");
12+
const WORKFLOW_PATH = join(ROOT, ".github", "workflows", "codebase-growth-guardrails.yaml");
13+
const STATIC_ACTION_PATH = join(ROOT, ".github", "actions", "ci-static-checks", "action.yaml");
14+
const CHECKOUT = "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1";
15+
const TEST_COMMAND =
16+
"set -euo pipefail\nnpx vitest run --project integration test/growth-guardrails.test.ts";
17+
const STATIC_COMMAND =
18+
"npx prek run --all-files --stage pre-commit \\\n --skip source-shape-test-budget \\\n --skip test-skills-yaml";
19+
20+
type Value = Record<string, unknown>;
21+
22+
function object(value: unknown): Value {
23+
return value && typeof value === "object" && !Array.isArray(value) ? (value as Value) : {};
24+
}
25+
26+
function array(value: unknown): unknown[] {
27+
return Array.isArray(value) ? value : [];
28+
}
29+
30+
function same(value: unknown, expected: unknown): boolean {
31+
return isDeepStrictEqual(value, expected);
32+
}
33+
34+
export function validateGrowthGuardrailsWorkflowBoundary(
35+
workflowSource = readFileSync(WORKFLOW_PATH, "utf8"),
36+
staticActionSource = readFileSync(STATIC_ACTION_PATH, "utf8"),
37+
): string[] {
38+
let workflow: Value;
39+
let action: Value;
40+
try {
41+
workflow = object(YAML.parse(workflowSource));
42+
action = object(YAML.parse(staticActionSource));
43+
} catch {
44+
return ["growth guardrail workflow configuration must be valid YAML"];
45+
}
46+
47+
const expectedWorkflow = {
48+
name: "Governance / Enforce Codebase Growth Limits",
49+
on: {
50+
pull_request_target: { types: ["opened", "reopened", "synchronize", "ready_for_review"] },
51+
},
52+
permissions: { contents: "read", "pull-requests": "read" },
53+
jobs: {
54+
"codebase-growth-guardrails": {
55+
name: "codebase-growth-guardrails",
56+
"runs-on": "ubuntu-latest",
57+
"timeout-minutes": 5,
58+
steps: [
59+
{
60+
name: "Check out the trusted base revision",
61+
uses: CHECKOUT,
62+
with: {
63+
ref: "${{ github.event.pull_request.base.sha }}",
64+
"persist-credentials": false,
65+
},
66+
},
67+
{
68+
name: "Install trusted dependencies",
69+
run: "npm ci --ignore-scripts --no-audit --no-fund",
70+
},
71+
{
72+
name: "Test codebase growth guardrails",
73+
env: {
74+
NEMOCLAW_GROWTH_PR: "1",
75+
GH_TOKEN: "${{ github.token }}",
76+
PR_NUMBER: "${{ github.event.pull_request.number }}",
77+
REPO: "${{ github.repository }}",
78+
BASE_SHA: "${{ github.event.pull_request.base.sha }}",
79+
HEAD_REPO: "${{ github.event.pull_request.head.repo.full_name }}",
80+
HEAD_SHA: "${{ github.event.pull_request.head.sha }}",
81+
},
82+
run: TEST_COMMAND + "\n",
83+
},
84+
],
85+
},
86+
},
87+
};
88+
const normalizedWorkflow: Value = { ...workflow, on: workflow.on ?? workflow.true };
89+
delete normalizedWorkflow.true;
90+
const errors: string[] = [];
91+
if (!same(normalizedWorkflow, expectedWorkflow)) {
92+
errors.push("growth guardrail workflow must match the reviewed trust boundary");
93+
}
94+
95+
const staticSteps = array(object(action.runs).steps).map(object);
96+
const namedStaticSteps = staticSteps.filter((step) => step.name === "Run static hook checks");
97+
if (
98+
namedStaticSteps.length !== 1 ||
99+
!same(namedStaticSteps[0], {
100+
name: "Run static hook checks",
101+
shell: "bash",
102+
run: STATIC_COMMAND + "\n",
103+
})
104+
) {
105+
errors.push("static action must retain the reviewed hook-check step");
106+
}
107+
if (JSON.stringify(action).includes("test-size:check")) {
108+
errors.push("static checks must not recursively invoke test-size:check");
109+
}
110+
return errors;
111+
}
112+
113+
const currentModule = fileURLToPath(import.meta.url);
114+
if (process.argv[1] === currentModule) {
115+
const errors = validateGrowthGuardrailsWorkflowBoundary();
116+
if (errors.length > 0) {
117+
errors.forEach((error) => console.error(error));
118+
process.exit(1);
119+
}
120+
console.log("Codebase growth guardrail workflow boundary passed.");
121+
}

scripts/checks/run.mts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ export const CHECKS: readonly CheckCommand[] = [
123123
command: TSX,
124124
args: ["scripts/checks/test-registration-boundary.mts"],
125125
},
126+
{
127+
name: "growth-guardrails-workflow-boundary",
128+
command: TSX,
129+
args: ["scripts/checks/growth-guardrails-workflow-boundary.mts"],
130+
},
126131
];
127132

128133
type RunChecksOptions = {

src/lib/messaging/AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Use the narrowest test that covers the changed surface:
9393
- Hook behavior: `npx vitest run src/lib/messaging/hooks src/lib/messaging/channels/<channel>/hooks`
9494
- Host/OpenShell application: `npx vitest run src/lib/messaging/applier`
9595
- Build-time render/install behavior: `npx vitest run test/messaging-build-applier.test.ts`
96-
- Onboard/channel CLI integration: `npx vitest run test/onboard-messaging.test.ts test/channels-add-preset.test.ts src/lib/onboard/messaging-channel-setup.test.ts`
96+
- Onboard/channel CLI integration: `npx vitest run test/onboard-messaging.test.ts test/channels/channels-add-preset.test.ts src/lib/onboard/messaging-channel-setup.test.ts`
9797

9898
Add focused negative tests for invalid credentials, unauthorized senders, denied network access, malformed configuration, and cleanup when those behaviors are in scope.
9999

@@ -119,5 +119,5 @@ LangChain Deep Agents Code is a terminal-oriented harness. NemoClaw does not run
119119
- **Invalid state.** A sandbox can be configured with an agent name that no channel manifest supports, or with stale `NEMOCLAW_MESSAGING_PLAN_B64` state from an earlier build. Without an explicit gate the channel-add path can still tear down the sandbox before failing at `dockerfile-patch.ts`, and rebuild can carry stale messaging plan data into an agent build path that does not consume it.
120120
- **Source boundary.** Channel manifests' `supportedAgents` lists are the single source of truth for whether a given agent supports messaging today, and which channels are available for it. Helpers in `utils.ts` derive the supported agent list from the active channel manifest registry, so `ChannelManifestRegistry.listAvailable`, `MessagingWorkflowPlanner.supportedChannelIds`, onboard state filtering, channel list, channel add/remove, and rebuild all share the same semantics. If no manifest supports the agent, deny or skip everywhere and clear stale staged plans.
121121
- **Source-fix constraint.** Expanding support for an agent requires per-channel `supportedAgents`, agent-side render and hook handlers in `applier/build/messaging-build-applier.mts`, the matching Dockerfile/build env plumbing when needed, and a runtime bridge/health path when public behavior claims channel readiness. Until that stack lands, the gate at the action boundary is the safe behavior: surface the unsupported-agent message in `addSandboxChannel`, clear the staged plan in `stageMessagingManifestPlanForRebuild`, and strip stale plans in `persistManifestChannelRemovePlan`.
122-
- **Regression tests.** `src/lib/messaging/utils.test.ts`, `src/lib/messaging/manifest/registry.test.ts`, and `src/lib/messaging/compiler/workflow-planner.test.ts` lock the helper and registry semantics. `src/lib/actions/sandbox/policy-channel-agent-gate.test.ts`, `src/lib/actions/sandbox/policy-channel-cleanup.test.ts`, `src/lib/actions/sandbox/rebuild-messaging-stage.test.ts`, and `src/lib/onboard/machine/handlers/sandbox.test.ts` cover the action, rebuild, and onboard-resume boundaries against stale or unsupported messaging plans. `test/channels-add-deepagents-rejection.test.ts` exercises the full DeepAgents `addSandboxChannel` boundary in a spawned Node process to prove no policy, provider, registry, credential, or rebuild call happens before the unsupported-agent exit.
122+
- **Regression tests.** `src/lib/messaging/utils.test.ts`, `src/lib/messaging/manifest/registry.test.ts`, and `src/lib/messaging/compiler/workflow-planner.test.ts` lock the helper and registry semantics. `src/lib/actions/sandbox/policy-channel-agent-gate.test.ts`, `src/lib/actions/sandbox/policy-channel-cleanup.test.ts`, `src/lib/actions/sandbox/rebuild-messaging-stage.test.ts`, and `src/lib/onboard/machine/handlers/sandbox.test.ts` cover the action, rebuild, and onboard-resume boundaries against stale or unsupported messaging plans. `test/channels/channels-add-deepagents-rejection.test.ts` exercises the full DeepAgents `addSandboxChannel` boundary in a spawned Node process to prove no policy, provider, registry, credential, or rebuild call happens before the unsupported-agent exit.
123123
- **Removal condition.** Drop the unsupported-agent gate only when every target agent is represented by channel manifest `supportedAgents` entries and `applier/build/messaging-build-applier.mts` resolves its render and runtime targets. At that point the unsupported-agent branch becomes unreachable for that agent and the action boundary can rely on planner-level validation alone.

src/lib/state/config-io.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import fs from "node:fs";
55
import os from "node:os";
66
import path from "node:path";
7-
import { afterEach, describe, expect, it } from "vitest";
7+
import { afterEach, describe, expect, it, vi } from "vitest";
88

99
import {
1010
ConfigCorruptError,
@@ -68,6 +68,55 @@ describe("config-io", () => {
6868
expect(() => ensureConfigDir(nestedDir)).toThrow(/symbolic link/);
6969
});
7070

71+
it("refuses to read a config file through a symlinked config directory", () => {
72+
const home = process.env.HOME || os.homedir();
73+
const tmp = fs.mkdtempSync(path.join(home, ".nemoclaw-test-"));
74+
tmpDirs.push(tmp);
75+
const attackerDir = path.join(tmp, "attacker");
76+
fs.mkdirSync(attackerDir, { mode: 0o700 });
77+
writeFileWithMode(
78+
path.join(attackerDir, "sandboxes.json"),
79+
JSON.stringify({ sandboxes: { planted: {} } }),
80+
0o600,
81+
);
82+
const symlinkPath = path.join(tmp, ".nemoclaw");
83+
fs.symlinkSync(attackerDir, symlinkPath);
84+
const plantedFile = path.join(symlinkPath, "sandboxes.json");
85+
86+
// The write path already refuses this exact path; the read path must agree.
87+
expect(() => writeConfigFile(plantedFile, { sandboxes: {} })).toThrow(/symbolic link/);
88+
expect(() => readConfigFile(plantedFile, null)).toThrow(/symbolic link/);
89+
});
90+
91+
it("reports permission failures while checking a read path with remediation", () => {
92+
const home = process.env.HOME || os.homedir();
93+
const tmp = fs.mkdtempSync(path.join(home, ".nemoclaw-test-"));
94+
tmpDirs.push(tmp);
95+
const configDir = path.join(tmp, ".nemoclaw");
96+
fs.mkdirSync(configDir, { mode: 0o700 });
97+
const configFile = path.join(configDir, "sandboxes.json");
98+
fs.writeFileSync(configFile, JSON.stringify({ sandboxes: {} }), { mode: 0o600 });
99+
100+
const realLstatSync = fs.lstatSync.bind(fs);
101+
const rejectInspection = (): never => {
102+
throw Object.assign(new Error("EACCES"), { code: "EACCES" });
103+
};
104+
const lstatSpy = vi.spyOn(fs, "lstatSync").mockImplementation((target, options) => {
105+
return path.resolve(String(target)) === path.resolve(configDir)
106+
? rejectInspection()
107+
: realLstatSync(target, options as never);
108+
});
109+
110+
try {
111+
const read = () => readConfigFile(configFile, null);
112+
expect(read).toThrow(ConfigPermissionError);
113+
expect(read).toThrow(/Cannot read config directory/);
114+
expect(read).toThrow(/sudo chown/);
115+
} finally {
116+
lstatSpy.mockRestore();
117+
}
118+
});
119+
71120
it("allows a normal directory (no symlinks)", () => {
72121
const home = process.env.HOME || os.homedir();
73122
const tmp = fs.mkdtempSync(path.join(home, ".nemoclaw-test-"));

src/lib/state/config-io.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ export class ConfigPermissionError extends Error {
156156
}
157157
}
158158

159+
class ConfigSymlinkError extends Error {
160+
constructor(message: string) {
161+
super(message);
162+
this.name = "ConfigSymlinkError";
163+
}
164+
}
165+
159166
/**
160167
* Reject a path if it — or any ancestor up to the user's home — is a symlink.
161168
* This prevents an attacker from planting e.g. ~/.nemoclaw as a symlink to an
@@ -185,7 +192,7 @@ export function rejectSymlinksOnPath(dirPath: string): void {
185192
const stat = fs.lstatSync(current);
186193
if (stat.isSymbolicLink()) {
187194
const target = fs.readlinkSync(current);
188-
throw new Error(
195+
throw new ConfigSymlinkError(
189196
`Refusing to use config directory: ${current} is a symbolic link ` +
190197
`(target: ${target}). This may indicate a symlink attack. ` +
191198
`Remove the symlink and retry: rm ${shellQuote(current)}`,
@@ -292,12 +299,21 @@ export function ensureConfigDir(dirPath: string): void {
292299
}
293300

294301
export function readConfigFile<T>(filePath: string, fallback: T): T {
302+
const dirPath = path.dirname(filePath);
295303
try {
296-
ensureConfigDir(path.dirname(filePath));
304+
ensureConfigDir(dirPath);
297305
} catch (error) {
298-
if (error instanceof ConfigPermissionError) {
306+
if (error instanceof ConfigSymlinkError || error instanceof ConfigPermissionError) {
299307
throw error;
300308
}
309+
const errnoError = error instanceof Error ? error : null;
310+
if (isPermissionError(errnoError)) {
311+
throw new ConfigPermissionError(
312+
`Cannot read config directory: ${dirPath}`,
313+
dirPath,
314+
toError(errnoError),
315+
);
316+
}
301317
// Directory doesn't exist and can't be created — fall through to let
302318
// readFileSync produce the appropriate ENOENT / fallback path.
303319
}

test/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ The project globs in `vitest.config.ts` must remain disjoint and exhaustive.
2828

2929
Choose the execution lane from the boundary that the test exercises.
3030
Within the integration project, group new tests by the behavior that owns the assertion.
31-
For example, `process-recovery/` owns sandbox process and forward recovery coverage.
31+
For example, `process-recovery/` owns sandbox process and forward recovery coverage, `channels/` owns channel lifecycle coverage, and `credentials/` owns host credential storage and reset coverage.
3232
Do not put an ordinary integration test in `e2e/` or `package-contract/`.
3333

3434
Run `npm run test:projects:check` after adding or moving a test.

test/channels-add-bridge-lifecycle.test.ts renamed to test/channels/channels-add-bridge-lifecycle.test.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@ import {
1616
removeSandboxChannel,
1717
startSandboxChannel,
1818
stopSandboxChannel,
19-
} from "../src/lib/actions/sandbox/policy-channel";
20-
import { policyChannelDependencies } from "../src/lib/actions/sandbox/policy-channel-dependencies";
21-
import * as processRecovery from "../src/lib/actions/sandbox/process-recovery";
22-
import * as runtime from "../src/lib/adapters/openshell/runtime";
23-
import * as store from "../src/lib/credentials/store";
24-
import * as gatewayRuntime from "../src/lib/gateway-runtime-action";
25-
import { MESSAGING_BRIDGE_PENDING_VALUE } from "../src/lib/onboard/messaging-bridge-provider";
26-
import * as policies from "../src/lib/policy";
27-
import * as onboardSession from "../src/lib/state/onboard-session";
28-
import type { SandboxEntry } from "../src/lib/state/registry";
29-
import * as registry from "../src/lib/state/registry";
19+
} from "../../src/lib/actions/sandbox/policy-channel";
20+
import { policyChannelDependencies } from "../../src/lib/actions/sandbox/policy-channel-dependencies";
21+
import * as processRecovery from "../../src/lib/actions/sandbox/process-recovery";
22+
import * as runtime from "../../src/lib/adapters/openshell/runtime";
23+
import * as store from "../../src/lib/credentials/store";
24+
import * as gatewayRuntime from "../../src/lib/gateway-runtime-action";
25+
import { MESSAGING_BRIDGE_PENDING_VALUE } from "../../src/lib/onboard/messaging-bridge-provider";
26+
import * as policies from "../../src/lib/policy";
27+
import * as onboardSession from "../../src/lib/state/onboard-session";
28+
import type { SandboxEntry } from "../../src/lib/state/registry";
29+
import * as registry from "../../src/lib/state/registry";
3030

3131
class ExitError extends Error {
3232
constructor(public readonly code: number | undefined) {
@@ -54,10 +54,10 @@ const GOOGLECHAT_ENV = {
5454
// handler is replaced with one that succeeds immediately — as if the operator
5555
// had already finished enrollment. Everything else in the add path runs real.
5656
type GateModule =
57-
typeof import("../src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate");
57+
typeof import("../../src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate");
5858

5959
vi.mock(
60-
"../src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate",
60+
"../../src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate",
6161
async (importOriginal) => {
6262
const actual = await importOriginal<GateModule>();
6363
return {

0 commit comments

Comments
 (0)