From 0855eee65931b8374ce3f459287807b07ceafd57 Mon Sep 17 00:00:00 2001 From: reesebuilt <126643625+reesepj@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:39:57 -0500 Subject: [PATCH] Upgrade js-yaml to 5.2.3 and harden the foreign YAML read js-yaml 5 loads with the YAML 1.2 core schema. Two of the resulting differences reach code that reads attacker-adjacent files. load() now throws on a stream that contains no document. The dashboard reads the agent harness's own config.yaml, a file Agentwall does not own, so a comment-only or half-written one would take the whole dashboard state build down rather than one panel. summarizeYamlSource returns the unknown summary on any parse failure, which covers malformed input too. Merge keys are no longer expanded, so a policy file that relies on << now fails schema validation and is rejected whole. FileBackedPolicyRuntime keeps the last good ruleset on any reload failure; the new cases pin that for parser-level rejection, not just schema-level rejection. @types/js-yaml is dropped because js-yaml 5 ships its own declarations. js-yaml is exact-pinned per the runtime dependency policy. Verified: require('js-yaml') resolves the CommonJS build and round-trips load and dump on node 24.14.1; dump output for the config and policy shapes is byte-identical to 4.3.1; the shipped examples carry no merge keys, YAML 1.1 booleans, leading-zero octals, or sexagesimal scalars. Signed-off-by: reesebuilt <126643625+reesepj@users.noreply.github.com> --- CHANGELOG.md | 2 + package-lock.json | 139 ++++++++----------------- package.json | 3 +- src/dashboard/state.ts | 14 ++- tests/dashboard-harness-config.test.ts | 55 ++++++++++ tests/policy-runtime.test.ts | 41 ++++++++ 6 files changed, 157 insertions(+), 97 deletions(-) create mode 100644 tests/dashboard-harness-config.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e113535..ead4db7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,8 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht - Pending approvals now have an explicit mixed-risk ordering test and panel-side sort to keep critical items pinned before older lower-priority reviews. - The approvals panel now shows a queue health summary with pending count, critical count, and oldest waiting item before the pending decision list. - Brand asset docs now point at the actual public asset path. +- Policy and config YAML now parses under the YAML 1.2 core schema on js-yaml 5. Merge keys (`<<`) are no longer expanded, so a policy file that relies on one is rejected whole and the last good ruleset stays in force instead of a partially assembled rule taking effect. Unquoted dates load as strings rather than `Date` objects, and a mapping with a complex key is rejected instead of having that key flattened into a lossy string. +- The dashboard runtime-context panel now degrades to "none" when the agent harness config file cannot be parsed, rather than failing the whole dashboard state build on a file Agentwall does not own. ## [0.1.0] - 2026-03-23 diff --git a/package-lock.json b/package-lock.json index 3eee9cc..ea1850c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "Apache-2.0", "dependencies": { "fastify": "5.11.0", - "js-yaml": "^4.1.0", + "js-yaml": "5.2.3", "pino": "^8.19.0", "zod": "4.4.3" }, @@ -19,7 +19,6 @@ }, "devDependencies": { "@jest/globals": "^30.4.1", - "@types/js-yaml": "^4.0.9", "@types/node": "^20.11.5", "jest": "^30.4.2", "ts-jest": "^29.4.12", @@ -305,13 +304,13 @@ } }, "node_modules/@babel/plugin-syntax-import-attributes": { - "version": "7.28.6", - "resolved": "https://registry.npmjs.org/@babel/plugin-syntax-import-attributes/-/plugin-syntax-import-attributes-7.28.6.tgz", - "integrity": "sha512-jiLC0ma9XkQT3TKJ9uYvlakm66Pamywo+qwL+oL8HJOvc6TWdZXVfhqJr8CCzbSGUAbDOzlGHJC1U+vRfLQDvw==", + "version": "7.29.7", + "resolved": "https://registry.npmjs.org/@babel/plugin-syntax-import-attributes/-/plugin-syntax-import-attributes-7.29.7.tgz", + "integrity": "sha512-zGYcYfq/WmZ4V+kBIXQon9dSSc8ircGZqw9ZaNhhGj9nZkeBu1jHLBDQqYYi5WA9uawvA2sIMbry2nCFhf5Djg==", "dev": true, "license": "MIT", "dependencies": { - "@babel/helper-plugin-utils": "^7.28.6" + "@babel/helper-plugin-utils": "^7.29.7" }, "engines": { "node": ">=6.9.0" @@ -772,9 +771,9 @@ } }, "node_modules/@istanbuljs/schema": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/@istanbuljs/schema/-/schema-0.1.3.tgz", - "integrity": "sha512-ZXRY4jNvVgSVQ8DL3LTcakaAtXwTVUxE81hslsyD2AtoXW/wVob10HkOJ1X/pAlcI7D+2YoZKg5do8G/w6RYgA==", + "version": "0.1.6", + "resolved": "https://registry.npmjs.org/@istanbuljs/schema/-/schema-0.1.6.tgz", + "integrity": "sha512-+Sg6GCR/wy1oSmQDFq4LQDAhm3ETKnorxN+y5nbLULOR3P0c14f2Wurzj3/xqPXtasLFfHd5iRFQ7AJt4KH2cw==", "dev": true, "license": "MIT", "engines": { @@ -1362,13 +1361,6 @@ "@types/istanbul-lib-report": "*" } }, - "node_modules/@types/js-yaml": { - "version": "4.0.9", - "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", - "integrity": "sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/node": { "version": "20.19.37", "resolved": "https://registry.npmjs.org/@types/node/-/node-20.19.37.tgz", @@ -1889,6 +1881,19 @@ "node": ">= 8" } }, + "node_modules/anymatch/node_modules/picomatch": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.2.tgz", + "integrity": "sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=8.6" + }, + "funding": { + "url": "https://github.com/sponsors/jonschlinkert" + } + }, "node_modules/arg": { "version": "4.1.3", "resolved": "https://registry.npmjs.org/arg/-/arg-4.1.3.tgz", @@ -2616,6 +2621,13 @@ "url": "https://github.com/sindresorhus/execa?sponsor=1" } }, + "node_modules/execa/node_modules/signal-exit": { + "version": "3.0.7", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", + "integrity": "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==", + "dev": true, + "license": "ISC" + }, "node_modules/exit-x": { "version": "0.2.2", "resolved": "https://registry.npmjs.org/exit-x/-/exit-x-0.2.2.tgz", @@ -2929,19 +2941,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/foreground-child/node_modules/signal-exit": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", - "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", - "dev": true, - "license": "ISC", - "engines": { - "node": ">=14" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" - } - }, "node_modules/fs.realpath": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", @@ -3545,19 +3544,6 @@ "fsevents": "^2.3.3" } }, - "node_modules/jest-haste-map/node_modules/picomatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", - "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://github.com/sponsors/jonschlinkert" - } - }, "node_modules/jest-leak-detector": { "version": "30.4.1", "resolved": "https://registry.npmjs.org/jest-leak-detector/-/jest-leak-detector-30.4.1.tgz", @@ -3610,19 +3596,6 @@ "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" } }, - "node_modules/jest-message-util/node_modules/picomatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", - "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://github.com/sponsors/jonschlinkert" - } - }, "node_modules/jest-mock": { "version": "30.4.1", "resolved": "https://registry.npmjs.org/jest-mock/-/jest-mock-30.4.1.tgz", @@ -3819,19 +3792,6 @@ "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" } }, - "node_modules/jest-util/node_modules/picomatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", - "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://github.com/sponsors/jonschlinkert" - } - }, "node_modules/jest-validate": { "version": "30.4.1", "resolved": "https://registry.npmjs.org/jest-validate/-/jest-validate-30.4.1.tgz", @@ -3924,9 +3884,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", - "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", + "version": "5.2.3", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-5.2.3.tgz", + "integrity": "sha512-n+mUVyUX5bVv7G/G2zyIHOhdxfuU1dY2NOFzTQUWiMUbFss8b57NFlgCCaggU78wSw5KVS9cllzeLyzyR+n5nw==", "funding": [ { "type": "github", @@ -3942,7 +3902,7 @@ "argparse": "^2.0.1" }, "bin": { - "js-yaml": "bin/js-yaml.js" + "js-yaml": "bin/js-yaml.mjs" } }, "node_modules/jsesc": { @@ -4428,13 +4388,13 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "2.3.2", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.2.tgz", - "integrity": "sha512-V7+vQEJ06Z+c5tSye8S+nHUfI51xoXIXjHQ99cQtKUkQqqO1kO/KCJUfZXuB47h/YBlDhah2H3hdUGXn8ie0oA==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", + "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", "engines": { - "node": ">=8.6" + "node": ">=12" }, "funding": { "url": "https://github.com/sponsors/jonschlinkert" @@ -4785,11 +4745,17 @@ } }, "node_modules/signal-exit": { - "version": "3.0.7", - "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", - "integrity": "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==", + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", "dev": true, - "license": "ISC" + "license": "ISC", + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } }, "node_modules/slash": { "version": "3.0.0", @@ -5574,19 +5540,6 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, - "node_modules/write-file-atomic/node_modules/signal-exit": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", - "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", - "dev": true, - "license": "ISC", - "engines": { - "node": ">=14" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" - } - }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", diff --git a/package.json b/package.json index 04b19f0..790c096 100644 --- a/package.json +++ b/package.json @@ -27,13 +27,12 @@ "license": "Apache-2.0", "dependencies": { "fastify": "5.11.0", - "js-yaml": "^4.1.0", + "js-yaml": "5.2.3", "pino": "^8.19.0", "zod": "4.4.3" }, "devDependencies": { "@jest/globals": "^30.4.1", - "@types/js-yaml": "^4.0.9", "@types/node": "^20.11.5", "jest": "^30.4.2", "ts-jest": "^29.4.12", diff --git a/src/dashboard/state.ts b/src/dashboard/state.ts index 810d07a..1c39c53 100644 --- a/src/dashboard/state.ts +++ b/src/dashboard/state.ts @@ -329,11 +329,21 @@ function yamlTopLevelKeys(source: unknown): string[] { } function summarizeYamlSource(absolutePath: string): { keys: string[]; modelDefault: string | null; modelProvider: string | null; skin: string | null } { + const unknownSummary = { keys: [] as string[], modelDefault: null, modelProvider: null, skin: null }; const text = readTextSafely(absolutePath); if (!text) { - return { keys: [], modelDefault: null, modelProvider: null, skin: null }; + return unknownSummary; + } + // This file belongs to the agent harness, not to us, so it is foreign input: it can hold + // nothing but comments, be half-written while an editor saves it, or be deliberately + // malformed by whoever can write into the agent home. A parse failure degrades one + // dashboard panel to "unknown" instead of taking the whole state build down with it. + let parsed: unknown; + try { + parsed = yaml.load(text); + } catch { + return unknownSummary; } - const parsed = yaml.load(text); return { keys: yamlTopLevelKeys(parsed), modelDefault: nestedStringValue(parsed, ["model", "default"]), diff --git a/tests/dashboard-harness-config.test.ts b/tests/dashboard-harness-config.test.ts new file mode 100644 index 0000000..57aed34 --- /dev/null +++ b/tests/dashboard-harness-config.test.ts @@ -0,0 +1,55 @@ +import { afterAll, describe, expect, it } from "@jest/globals"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +/** + * The dashboard reads the agent harness's own config.yaml to describe the runtime context. + * That file belongs to the harness, not to Agentwall, so its bytes are foreign input: it can + * hold nothing but comments, be caught half-written while an editor saves it, or be + * deliberately malformed by anyone who can write into the agent home. A YAML parse failure + * there costs one panel its detail and nothing more, because an attacker who can blind the + * whole operator view can then act unobserved. + */ +const agentHome = fs.mkdtempSync(path.join(os.tmpdir(), "agentwall-harness-home-")); +process.env["AGENTWALL_AGENT_HOME"] = agentHome; + +afterAll(() => { + fs.rmSync(agentHome, { recursive: true, force: true }); +}); + +// Loaded through await import, not a static import, because state.ts resolves +// AGENTWALL_AGENT_HOME once at module load and static imports are evaluated before the +// assignment above runs. This test exercises that module-loading boundary on purpose. +async function snapshotFacts(harnessConfig: string): Promise> { + fs.writeFileSync(path.join(agentHome, "config.yaml"), harnessConfig); + const { RuntimeState } = await import("../src/dashboard/state"); + const { loadConfig } = await import("../src/config"); + const snapshot = new RuntimeState(loadConfig("examples/config.yaml")).getSnapshot(0); + const entry = snapshot.knowledgeBase.entries.find((item) => item.id === "system_environment"); + expect(entry).toBeDefined(); + return entry?.facts ?? []; +} + +describe("dashboard runtime context with an unreadable harness config", () => { + it.each([ + ["a document with no content", "# nothing configured yet\n"], + ["unparseable YAML", "model:\n\tdefault: gpt\n"], + ["a truncated flow collection", "model: [\n"], + ])("degrades one panel when the harness config is %s", async (_label, contents) => { + const facts = await snapshotFacts(contents); + + // The file is present and was read; only the parse of it failed. + expect(facts.find((fact) => fact.label === "Config file")?.value).toBe("configured"); + expect(facts.find((fact) => fact.label === "Config keys")?.value).toBe("none"); + expect(facts.find((fact) => fact.label === "Model")?.value).toBe("unknown"); + }); + + it("still reports keys when the harness config parses", async () => { + const facts = await snapshotFacts("model:\n default: gpt-5\ndisplay:\n skin: dark\n"); + + expect(facts.find((fact) => fact.label === "Config keys")?.value).toBe("model, display"); + expect(facts.find((fact) => fact.label === "Model")?.value).toBe("gpt-5"); + expect(facts.find((fact) => fact.label === "Display skin")?.value).toBe("dark"); + }); +}); diff --git a/tests/policy-runtime.test.ts b/tests/policy-runtime.test.ts index 78aec6c..3f4adfe 100644 --- a/tests/policy-runtime.test.ts +++ b/tests/policy-runtime.test.ts @@ -87,6 +87,47 @@ rules: expect(failedReload.error).toBeDefined(); expect(runtime.getRules().map((rule) => rule.id)).toEqual(["custom:allow-second"]); }); + + // A policy file is attacker-adjacent input: anyone who can write into the policy path can + // decide what the parser sees. A file the YAML parser rejects outright must be rejected + // whole, never partially applied, and the last good ruleset must stay in force so a + // malformed write cannot silently disarm the running policy. + it.each([ + ["unparseable YAML", "version: \"1\"\nrules:\n\t- id: \"custom:tabbed\"\n"], + ["a truncated flow collection", "version: \"1\"\nrules: [\n"], + ["a document with no content", "# every line here is a comment\n"], + ])("rejects %s and keeps the last good rules", (_label, contents) => { + const { dir, policyPath } = createPolicyDir(); + tempDirs.push(dir); + + writePolicy(policyPath, ` +version: "1" +rules: + - id: "custom:allow-good" + description: "Allow known host" + plane: "network" + match: + type: "hostname-equals" + values: ["api.good.example"] + decision: "allow" + riskLevel: "low" + reason: "Known host allowed" +`); + + const runtime = new FileBackedPolicyRuntime(policyPath, { + watch: false, + logger: { error: () => {}, warn: () => {} }, + }); + expect(runtime.getRules().map((rule) => rule.id)).toEqual(["custom:allow-good"]); + + writePolicy(policyPath, contents); + + const failedReload = runtime.reload(); + expect(failedReload.reloaded).toBe(false); + expect(failedReload.error).toBeInstanceOf(Error); + expect(runtime.getRules().map((rule) => rule.id)).toEqual(["custom:allow-good"]); + expect(runtime.getDeclarativeRules().map((rule) => rule.id)).toEqual(["custom:allow-good"]); + }); }); describe("buildServer policy reload", () => {