Skip to content

fix(denkraum): enforce full startup config and physical state boundary #750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 2 commits into from
Oct 9, 2026
Merged

fix(denkraum): enforce full startup config and physical state boundary #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35dd426
fix(denkraum): enforce full startup config and physical state boundary
Cuarroc Oct 8, 2026
b43e87c
fix(denkraum): resolve Windows aliases and validate webhook preflight
Cuarroc Oct 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 48 additions & 10 deletions tools/denkraum/config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { isAbsolute, relative, resolve, sep } from "node:path";
import { dirname, isAbsolute, relative, resolve, sep } from "node:path";
import { lstatSync, realpathSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { createRequire } from "node:module";
import { validId } from "./store/model.mjs";

// Pure preflight: no file access, server start, or secret-source assumptions.
const isInside = (root, path) => {
const fromRoot = relative(root, path);
return !isAbsolute(fromRoot) && fromRoot !== ".." && !fromRoot.startsWith(`..${sep}`);
};
function physicalAncestor(path) {
for (;;) {
try { return realpathSync.native(path); }
catch (error) {
if (error.code !== "ENOENT" || dirname(path) === path) throw error;
// An existing broken link is not an absent directory: do not skip it.
if (lstatSync(path, { throwIfNoEntry: false })) throw error;
path = dirname(path);
}
}
}

// Read-only preflight; no server start or secret-source assumptions.
// Only config contains values; errors and notification reasons are safe to print.
export function loadStartConfig(env, { repoRoot }) {
const errors = [];
Expand All @@ -24,27 +42,46 @@ export function loadStartConfig(env, { repoRoot }) {
} else if (webhookSecret === rootReceiptToken) {
error("WEBHOOK_SECRET", "must differ from DECISION_DESK_ROOT_RECEIPT_TOKEN");
}
if (rootAgentId === null) error("ROOT_AGENT_ID", "required");
else if (!validId(rootAgentId) || rootAgentId !== rootAgentId.trim()) {
error("ROOT_AGENT_ID", "use 1–80 letters, digits, underscores or hyphens; start with a letter or digit");
}

let statePath = null;
if (typeof state !== "string" || !isAbsolute(state) || state.includes("\0")) {
error("STATE", "required; use an absolute path");
} else {
statePath = resolve(state);
const fromRepo = relative(resolve(repoRoot), statePath);
if (process.platform === "win32" && statePath.startsWith("\\\\")) {
error("STATE", "Windows namespace and UNC state paths are not supported");
} else if (!isAbsolute(fromRepo) && fromRepo !== ".." && !fromRepo.startsWith(`..${sep}`)) {
} else if (isInside(resolve(repoRoot), statePath)) {
error("STATE", "must be outside the repository after path resolution");
} else {
try {
if (isInside(realpathSync.native(repoRoot), physicalAncestor(statePath))) {
error("STATE", "must be outside the repository after physical path resolution");
}
} catch {
error("STATE", "physical path resolution failed");
}
}
}
if (rawPort !== undefined && (typeof rawPort !== "string" || !rawPort
|| /[^0-9]/u.test(rawPort) || !Number.isInteger(port) || port < 1 || port > 65535)) {
error("PORT", "must be an integer from 1 to 65535");
}
const notifications = {
enabled: rootAgentId !== null,
reason: rootAgentId === null ? "DECISION_DESK_ROOT_AGENT_ID: missing; notifications disabled" : null,
};
const webhookUrl = env.DECISION_DESK_WEBHOOK_URL;
if (webhookUrl) {
let endpoint;
try { endpoint = new URL(webhookUrl); } catch { /* Fixed diagnostic below; never expose the URL. */ }
if (!endpoint || endpoint.protocol !== "https:" || endpoint.hostname !== "agentsroom.dev" || endpoint.port
|| endpoint.username || endpoint.password || endpoint.search || endpoint.hash
|| !/^\/api\/triggers\/t_[a-f0-9]+$/.test(endpoint.pathname)) {
error("WEBHOOK_URL", "must be an HTTPS AgentsRoom trigger URL without credentials, port, query or fragment");
}
}
const notifications = webhookUrl ? { enabled: true, reason: null }
: { enabled: false, reason: "DECISION_DESK_WEBHOOK_URL not set" };
return {
ok: errors.length === 0,
config: errors.length ? null : { rootReceiptToken, webhookSecret, rootAgentId, statePath, port, notifications },
Expand All @@ -53,7 +90,7 @@ export function loadStartConfig(env, { repoRoot }) {
}

let isCliEntry = import.meta.main;
// Node 24.0/24.1 lack import.meta.main; resolve only the CLI entry, never STATE.
// Node 24.0/24.1 lack import.meta.main; resolve the CLI entry for those versions.
if (isCliEntry === undefined && process.argv[1]
&& !process.execArgv.some((arg) => /^(?:--(?:eval|print)(?:=|$)|-[ep])/u.test(arg))) {
try {
Expand All @@ -74,7 +111,8 @@ if (isCliEntry) {
for (const suffix of ["ROOT_RECEIPT_TOKEN", "WEBHOOK_SECRET", "STATE", "PORT"]) {
console.log(`DECISION_DESK_${suffix}: valid`);
}
console.log(result.config.notifications.reason ?? "DECISION_DESK_ROOT_AGENT_ID: notifications enabled");
const { enabled, reason } = result.config.notifications;
console.log(enabled ? "DECISION_DESK_WEBHOOK_URL: notifications enabled" : `notifications disabled: ${reason}`);
}
process.exitCode = result.ok ? 0 : 1;
}
Expand Down
98 changes: 85 additions & 13 deletions tools/denkraum/config.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import { mkdtempSync, symlinkSync } from "node:fs";
import { mkdirSync, mkdtempSync, realpathSync, symlinkSync } from "node:fs";
import { tmpdir } from "node:os";
import { resolve, join, dirname, toNamespacedPath, win32 } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { loadStartConfig } from "./config.mjs";

const repoRoot = resolve("synthetic-repository");
const fixtureRoot = mkdtempSync(join(tmpdir(), "denkraum-config-"));
const repoRoot = join(fixtureRoot, "synthetic-repository");
mkdirSync(repoRoot);
const name = (suffix) => `DECISION_DESK_${suffix}`;
const validEnv = () => ({
[name("ROOT_RECEIPT_TOKEN")]: "a".repeat(32),
[name("WEBHOOK_SECRET")]: "b".repeat(32),
[name("ROOT_AGENT_ID")]: "synthetic-" + "agent",
[name("STATE")]: resolve("synthetic-state", "ledger.json"),
[name("STATE")]: join(fixtureRoot, "synthetic-state", "ledger.json"),
});
const load = (env) => loadStartConfig(env, { repoRoot });
function refuses(suffix, values) {
Expand All @@ -36,7 +38,7 @@ test("loads valid config with default and explicit ports", () => {
webhookSecret: env[name("WEBHOOK_SECRET")],
rootAgentId: env[name("ROOT_AGENT_ID")],
statePath: env[name("STATE")], port: 4791,
notifications: { enabled: true, reason: null },
notifications: { enabled: false, reason: "DECISION_DESK_WEBHOOK_URL not set" },
});
for (const port of ["1", "65535"]) {
assert.equal(load({ ...env, [name("PORT")]: port }).config.port, Number(port));
Expand All @@ -61,14 +63,42 @@ test("refuses identical root and webhook secrets", () => {
refuses("WEBHOOK_SECRET", [validEnv()[name("ROOT_RECEIPT_TOKEN")]]);
});

test("disables notifications without an agent id and never falls back", () => {
test("missing root agent id is a start error", () => {
for (const missing of [undefined, "", " "]) {
const result = load({ ...validEnv(), [name("ROOT_AGENT_ID")]: missing });
assert.equal(result.ok, true);
assert.equal(result.config.rootAgentId, null);
assert.deepEqual(result.config.notifications, {
enabled: false, reason: "DECISION_DESK_ROOT_AGENT_ID: missing; notifications disabled",
});
assert.equal(result.ok, false);
assert.equal(result.config, null);
assert.deepEqual(result.errors.map((error) => error.name), [name("ROOT_AGENT_ID")]);
assert.equal(result.errors[0].reason, "required");
}
});

test("rejects root agent ids that the store cannot use", () => {
refuses("ROOT_AGENT_ID", [" leading", "trailing ", "invalid/id", "a".repeat(81), "id\n"]);
});

test("STATE through a junction/symlink into the repository is rejected", (t) => {
const fixture = mkdtempSync(join(tmpdir(), "denkraum-state-phys-"));
const fakeRepo = join(fixture, "repo");
const outside = join(fixture, "outside");
mkdirSync(fakeRepo);
mkdirSync(outside);
try {
symlinkSync(fakeRepo, join(outside, "into-repo"), process.platform === "win32" ? "junction" : "dir");
} catch (error) {
if (error.code === "EPERM") return t.skip("symlinks need extra rights on this machine");
throw error;
}
for (const suffix of [["ledger.json"], ["missing", "nested", "ledger.json"]]) {
const stateThroughLink = join(outside, "into-repo", ...suffix);
const result = loadStartConfig({ ...validEnv(), [name("STATE")]: stateThroughLink }, { repoRoot: fakeRepo });
assert.equal(result.ok, false);
assert.equal(result.config, null);
assert.deepEqual(result.errors.map((error) => error.name), [name("STATE")]);
assert.match(result.errors[0].reason, /outside the repository/);
for (const value of [stateThroughLink, fakeRepo, fixture]) {
assert.equal(result.errors[0].reason.includes(value), false);
}
}
});

Expand Down Expand Up @@ -134,13 +164,13 @@ test("check CLI never prints synthetic environment values", () => {
env[name("WEBHOOK_SECRET")] += "\n";
env[name("STATE")] = join(actualRoot, "private-state");
env[name("PORT")] = "invalid-" + "port";
} else delete env[name("ROOT_AGENT_ID")];
}
const child = spawnSync(process.execPath, [script, "--check"], { env, encoding: "utf8" });
assert.equal(child.error, undefined);
assert.equal(child.status, invalid ? 1 : 0);
const output = child.stdout + child.stderr;
for (const value of Object.values(env)) assert.equal(output.includes(value), false);
if (!invalid) assert.match(output, /DECISION_DESK_ROOT_AGENT_ID: missing; notifications disabled/);
if (!invalid) assert.match(output, /notifications disabled: DECISION_DESK_WEBHOOK_URL not set/);
if (invalid) for (const suffix of ["ROOT_RECEIPT_TOKEN", "WEBHOOK_SECRET", "STATE", "PORT"]) {
assert.ok(output.includes(name(suffix)));
}
Expand Down Expand Up @@ -170,7 +200,7 @@ test("checks config through a real CLI directory alias without running on import
assert.equal(valid.stderr, "");
assert.equal(valid.stdout, ["ROOT_RECEIPT_TOKEN", "WEBHOOK_SECRET", "STATE", "PORT"]
.map((suffix) => `${name(suffix)}: valid\n`).join("")
+ "DECISION_DESK_ROOT_AGENT_ID: notifications enabled\n");
+ "notifications disabled: DECISION_DESK_WEBHOOK_URL not set\n");
const usage = spawnSync(process.execPath, [entry], { env, encoding: "utf8" });
assert.equal(usage.error, undefined);
assert.equal(usage.status, 1);
Expand All @@ -186,3 +216,45 @@ test("checks config through a real CLI directory alias without running on import
assert.equal(imported.stdout + imported.stderr, "");
}
});

test("physical STATE checks resolve repository aliases and fail closed on broken links", () => {
const alias = join(fixtureRoot, "repo-alias");
const broken = join(fixtureRoot, "broken");
symlinkSync(repoRoot, alias, process.platform === "win32" ? "junction" : "dir");
symlinkSync(join(fixtureRoot, "missing"), broken, process.platform === "win32" ? "junction" : "dir");
Comment on lines +223 to +224

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle link-creation denial in the physical-path test

On Windows environments where creating junctions requires unavailable privileges, either symlinkSync call throws EPERM and aborts the entire test file instead of reporting this case as unsupported. The new junction test above already catches and skips on EPERM; apply the same handling here so restricted Windows runners can complete the remaining configuration tests.

Useful? React with 👍 / 👎.

for (const state of [join(alias, "state.json"), join(broken, "state.json")]) {
refuses("STATE", [state]);
}
const result = loadStartConfig({ ...validEnv(), [name("STATE")]: join(repoRoot, "state.json") }, { repoRoot: alias });
assert.equal(result.ok, false);
assert.equal(result.config, null);
assert.match(result.errors[0].reason, /outside the repository/);
});

test("STATE through Windows 8.3 repository aliases is rejected", (t) => {
if (process.platform !== "win32") return t.skip("Windows-only 8.3 short-name resolution");
const long = realpathSync.native(repoRoot);
const child = spawnSync("cmd.exe", ["/d", "/c", 'for %I in ("%DENKRAUM_TEST_REPO%") do @echo %~sI'],
{ env: { ...process.env, DENKRAUM_TEST_REPO: long }, encoding: "utf8", windowsVerbatimArguments: true });
assert.equal(child.error, undefined);
assert.equal(child.status, 0);
const short = child.stdout.trim();
assert.ok(short);
if (short.toLowerCase() === long.toLowerCase()) return t.skip("8.3 short names are disabled on the fixture volume");
for (const [root, alias] of [[long, short], [short, long]]) {
const result = loadStartConfig({ ...validEnv(), [name("STATE")]: join(alias, "missing", "ledger.json") }, { repoRoot: root });
assert.equal(result.ok, false);
assert.equal(result.config, null);
assert.deepEqual(result.errors, [{ name: name("STATE"), reason: "must be outside the repository after physical path resolution" }]);
}
});

test("check reports notifications disabled without a webhook URL", () => {
const env = validEnv();
assert.deepEqual(load(env).config.notifications, { enabled: false, reason: "DECISION_DESK_WEBHOOK_URL not set" });
const child = spawnSync(process.execPath, [fileURLToPath(new URL("config.mjs", import.meta.url)), "--check"], { env, encoding: "utf8" });
assert.equal(child.error, undefined);
assert.equal(child.status, 0);
assert.doesNotMatch(child.stdout, /notifications enabled/);
assert.match(child.stdout, /notifications disabled: DECISION_DESK_WEBHOOK_URL not set/);
});
Loading
Loading