From e78f65539a4ca84f56ac8114137a2472aa7d04a1 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 23:39:51 -0500 Subject: [PATCH 1/2] test(site): run the shipped docs/site.js in CI, with hostile payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit site.js is the file GitHub Pages actually serves, and no Go test could see it. The gap showed itself as soon as the file was executed rather than read: the URL check was a startsWith on the raw string, so .../releases/download/v9/../../../evil -> https://github.com/DevCop95/evil survived it. A wrong API field name, a broken URL check or a renamed element id in index.html would all fail silently too: the reader gets an empty download table and no error on either side. docs/site.test.js runs the real file against a minimal DOM stub, no dependencies and no network: A the real API shape (7 assets) fills every column, including the SHA-256 from checksums.txt and the em dash for assets it does not cover B a hostile payload (javascript: URL, foreign host, ../ traversal, raw HTML asset name) yields exactly one row — the on-path one — and the HTML name lands in textContent, not markup C/D nothing usable, and a 403, both fall back to the pinned list and both name the actual reason in the status line E checksums.txt unreachable: the table still renders, hashless F the hand-written fallback table in index.html keeps matching the published checksums.txt and the API byte counts humanised (5.8 MiB / 5.4 MiB) The element ids come from index.html, so renaming one there fails here instead of silently emptying the page. A static check also fails if any code path starts building markup from strings. Fixtures are trimmed copies of a real release response, so the gate cannot go red because GitHub was slow. SITE_TEST_LIVE=1 runs the same scenarios against the live API; CI never sets it. Verified under node 20 (what the job pins) and node 24. New required check: "site js test". --- .github/workflows/ci.yml | 21 +++ docs/site.test.js | 294 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+) create mode 100644 docs/site.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db2e3b4..91bd0ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,27 @@ jobs: - name: unit tests (platform-neutral packages) run: go test ./internal/token ./internal/audit ./internal/ui ./internal/server + # The docs site's JS (docs/site.js) is the file GitHub Pages actually serves, + # and no Go test can see it. A wrong API field name, a broken URL check or a + # renamed element id in index.html all fail *silently*: the reader gets an + # empty download table and no error on either side. That is not hypothetical — + # the first shipped URL check accepted ".../releases/download/v9/../../../evil", + # which resolves to https://github.com/DevCop95/evil, and it was found by + # running the file, not reading it. So the shipped file is run in node against + # a stub DOM, with hostile payloads. No install, no network: fixtures are + # trimmed copies of a real API response, so this gate cannot go red because + # GitHub was slow. + site-test: + name: site js test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 + with: + node-version: 20 + - name: run docs/site.test.js against docs/site.js + run: node docs/site.test.js + # Fuzzing on every push/PR. Two groups of targets: # * the module parsers — registry hives, KDC replies, payload envelopes — # which read bytes this tool did not author; diff --git a/docs/site.test.js b/docs/site.test.js new file mode 100644 index 0000000..964404c --- /dev/null +++ b/docs/site.test.js @@ -0,0 +1,294 @@ +// Runs docs/site.js — the file Pages actually serves — against a minimal DOM +// stub, so what it reads from the GitHub API and the URL check it applies are +// asserted instead of assumed. +// +// Why this exists: site.js renders the download table, and no Go test can see +// it. A renamed field, a broken URL check or an element id changed in +// index.html all fail *quietly* — the reader gets an empty table and no error +// on either side. That is not hypothetical: the first shipped version of the +// URL check accepted +// +// https://github.com/DevCop95/pullgoscript/releases/download/v9/../../../evil +// +// which resolves to https://github.com/DevCop95/evil. Running the file found +// it; reading it did not. So the file gets run. +// +// No dependencies and no network: `node docs/site.test.js` in CI, nothing to +// install. The fixtures are trimmed copies of a real +// /repos/DevCop95/pullgoscript/releases/latest response (v2.0.2, published +// 2026-10-17) and of that release's checksums.txt, so the shapes are the ones +// GitHub actually sends. SITE_TEST_LIVE=1 hits the live API instead; CI never +// does, so a GitHub hiccup cannot turn this gate red. +"use strict"; + +const fs = require("fs"); +const path = require("path"); +const assert = require("assert/strict"); + +const DIR = __dirname; +const SRC = fs.readFileSync(path.join(DIR, "site.js"), "utf8"); +const HTML = fs.readFileSync(path.join(DIR, "index.html"), "utf8"); + +const ORIGIN = "https://github.com"; +const ASSET_PATH = "/DevCop95/pullgoscript/releases/download/"; +const PREFIX = ORIGIN + ASSET_PATH; + +let checks = 0; +function ok(label, detail) { + checks++; + console.log(" ok " + label + (detail !== undefined ? " (" + detail + ")" : "")); +} +function eq(label, actual, expected) { + checks++; + assert.deepEqual( + actual, + expected, + label + + "\n expected: " + JSON.stringify(expected) + + "\n actual: " + JSON.stringify(actual) + ); + console.log(" ok " + label); +} + +// --- fixtures: real values, not invented ones ---------------------------- +// Sizes are the byte counts the API reported, which is also what pins the +// human-readable column: 6131480 must render as "5.8 MiB", the same string the +// static fallback table in index.html carries by hand. + +const CHECKSUMS = [ + "90b2e6e5794ed17892ccded42df81ea74840ca40b50370bd9fc7e117d9cff3c8 obliteratus_2.0.2_windows_amd64.zip", + "ce40c67281c31eb540e8f4639894bf471b110f1fb35136ed413344dd64477c66 obliteratus_2.0.2_windows_amd64.zip.sbom.json", + "ae47be1d2bbce867b6e5f08de35be2e3d90649ffa8bf9c7bdecc1930cb077160 obliteratus_2.0.2_windows_arm64.zip", + "68b9ab079ed67a5b5dba381ec2cff53f3b9dcf6ebe74de1a57a778d442a3662c obliteratus_2.0.2_windows_arm64.zip.sbom.json", +].join("\n") + "\n"; + +const RELEASE = { + tag_name: "v2.0.2", + published_at: "2026-10-17T13:36:41Z", + assets: [ + { name: "checksums.txt", size: 428, browser_download_url: PREFIX + "v2.0.2/checksums.txt" }, + { name: "checksums.txt.pem", size: 3268, browser_download_url: PREFIX + "v2.0.2/checksums.txt.pem" }, + { name: "checksums.txt.sig", size: 96, browser_download_url: PREFIX + "v2.0.2/checksums.txt.sig" }, + { name: "obliteratus_2.0.2_windows_amd64.zip", size: 6131480, browser_download_url: PREFIX + "v2.0.2/obliteratus_2.0.2_windows_amd64.zip" }, + { name: "obliteratus_2.0.2_windows_amd64.zip.sbom.json", size: 26548, browser_download_url: PREFIX + "v2.0.2/obliteratus_2.0.2_windows_amd64.zip.sbom.json" }, + { name: "obliteratus_2.0.2_windows_arm64.zip", size: 5622388, browser_download_url: PREFIX + "v2.0.2/obliteratus_2.0.2_windows_arm64.zip" }, + { name: "obliteratus_2.0.2_windows_arm64.zip.sbom.json", size: 26548, browser_download_url: PREFIX + "v2.0.2/obliteratus_2.0.2_windows_arm64.zip.sbom.json" }, + ], +}; + +// --- the DOM the page would build, plus a trap for unsafe hrefs ---------- + +let violations = []; + +function makeEl(tag) { + const el = { + tagName: tag, + children: [], + textContent: "", + className: "", + hidden: false, + _href: null, + appendChild(c) { el.children.push(c); return c; }, + removeChild(c) { el.children = el.children.filter(x => x !== c); return c; }, + get firstChild() { return el.children[0] || null; }, + }; + Object.defineProperty(el, "href", { + get() { return el._href; }, + set(v) { + if (v === null || v === undefined) { + // A real DOM turns `a.href = null` into the string "null" and the link + // points at /pullgoscript/null. Nothing should ever assign it. + violations.push("href assigned " + v); + } else if (String(v).indexOf(PREFIX) !== 0) { + violations.push("href outside the download path: " + v); + } + el._href = v; + }, + }); + return el; +} + +// The ids are read from index.html, not hard-coded: rename one there and +// site.js returns at its guard, which this test reports as an empty table +// rather than a passing run. +const idsInHTML = new Set([...HTML.matchAll(/\bid="([^"]+)"/g)].map(m => m[1])); + +async function run(payload, opts) { + opts = opts || {}; + violations = []; + const dom = { "dl-live": makeEl("div"), "dl-status": makeEl("p"), "dl-fallback": makeEl("table") }; + const fetched = []; + + const document = { + getElementById: id => (idsInHTML.has(id) ? dom[id] || null : null), + createElement: makeEl, + }; + const fetch = url => { + fetched.push(url); + if (opts.httpStatus && url.indexOf("/releases/latest") !== -1) { + return Promise.resolve({ ok: false, status: opts.httpStatus, json: () => Promise.reject(new Error("no body")), text: () => Promise.resolve("") }); + } + if (url.indexOf("checksums.txt") !== -1) { + if (opts.sumsBroken) return Promise.reject(new Error("network down")); + return Promise.resolve({ ok: true, status: 200, text: () => Promise.resolve(opts.sumsText || "") }); + } + return Promise.resolve({ ok: true, status: 200, json: () => Promise.resolve(payload) }); + }; + + new Function("document", "fetch", SRC)(document, fetch); + await new Promise(r => setTimeout(r, 120)); + + const table = dom["dl-live"].children[0] || null; + const tbody = table ? table.children.find(c => c.tagName === "tbody") : null; + const rows = tbody ? tbody.children : []; + return { + table, rows, fetched, + fallbackHidden: dom["dl-fallback"].hidden, + status: dom["dl-status"].textContent, + caption: table && table.children[0] ? table.children[0].textContent : "", + hrefs: rows.map(r => r.children[0].children[0]._href), + names: rows.map(r => r.children[0].children[0].textContent), + sizes: rows.map(r => r.children[1].textContent), + hashes: rows.map(r => r.children[2].textContent), + }; +} + +function live() { + return process.env.SITE_TEST_LIVE === "1"; +} + +async function loadLive(what) { + const r = await fetch("https://api.github.com/repos/DevCop95/pullgoscript/releases/latest", { + headers: { Accept: "application/vnd.github+json" }, + }); + if (!r.ok) throw new Error("the API answered " + r.status); + const release = await r.json(); + if (what === "release") return release; + const sums = (release.assets || []).find(a => a.name === "checksums.txt"); + if (!sums) return ""; + const t = await fetch(sums.browser_download_url); + return t.ok ? t.text() : ""; +} + +(async function main() { + console.log("site.test.js — " + (live() ? "live API" : "fixtures") + "\n"); + + // --- static discipline ------------------------------------------------- + // The comment in site.js promises API strings are never written as markup. + // This keeps that promise from being quietly dropped in a later edit. The + // check runs on code only (comments stripped), because the rationale comment + // at the top of site.js itself names innerHTML as the mistake to avoid. + const codeOnly = SRC + .replace(/\/\*[\s\S]*?\*\//g, "") + .split("\n") + .filter(l => !/^\s*\/\//.test(l)) + .join("\n"); + assert.ok(!/innerHTML|outerHTML|insertAdjacentHTML/.test(codeOnly), + "site.js must not build markup from strings (innerHTML found in code)"); + ok("site.js contains no innerHTML/outerHTML/insertAdjacentHTML (code only)"); + + const needed = ["dl-live", "dl-status", "dl-fallback"]; + needed.forEach(id => { + assert.ok(idsInHTML.has(id), "index.html is missing id=\"" + id + "\""); + }); + ok("index.html carries every element id site.js looks up", needed.join(", ")); + + const release = live() ? await loadLive("release") : RELEASE; + const sumsText = live() ? await loadLive("checksums") : CHECKSUMS; + + // --- A. the real shape ------------------------------------------------- + console.log("\nA. real payload + real checksums.txt"); + const a = await run(release, { sumsText }); + const assetNames = (release.assets || []).map(x => x.name); + eq("every asset in the release gets a row", a.rows.length, assetNames.length); + eq("fallback list is hidden once the live table renders", a.fallbackHidden, true); + eq("no href outside the download path", violations.length, 0); + assert.ok(a.caption.indexOf(release.tag_name) !== -1, "caption must name the tag"); + ok("caption names the release", a.caption); + + // The hash column is filled from checksums.txt, keyed by asset name: this is + // the check that would have caught a misnamed field. + const amd64 = a.names.findIndex(n => /_windows_amd64\.zip$/.test(n)); + if (amd64 >= 0) { + const published = new Map( + sumsText.trim().split("\n").map(l => l.trim().split(/\s+/)).filter(p => p.length >= 2) + .map(p => [p[1], p[0]]) + ); + eq("SHA-256 column matches checksums.txt for " + a.names[amd64], + a.hashes[amd64], published.get(a.names[amd64])); + eq("size column is the byte count, humanised", a.sizes[amd64], "5.8 MiB"); + } else { + ok("no amd64 zip in this release, hash-column check skipped"); + } + + // Assets with no line in checksums.txt must read as "no hash", not as empty + // (an empty cell looks like a layout bug; an em dash reads as intentional). + const txt = a.names.findIndex(n => n === "checksums.txt"); + if (txt >= 0) eq("asset absent from checksums.txt shows an em dash", a.hashes[txt], "\u2014"); + + // --- B. hostile payload ------------------------------------------------ + console.log("\nB. hostile payload"); + const evil = { + tag_name: "v9.9.9", + published_at: "2026-01-01T00:00:00Z", + assets: [ + { name: "", size: null, browser_download_url: PREFIX + "v9.9.9/x" }, + { name: "js", size: 1, browser_download_url: "javascript:alert(1)" }, + { name: "foreign", size: 1, browser_download_url: "https://evil.com/pullgoscript/releases/download/v9/x" }, + { name: "traversal", size: 1, browser_download_url: PREFIX + "v9.9.9/../../../evil" }, + ], + }; + const b = await run(evil, { sumsText: "" }); + eq("only the one asset on the real download path survives", b.names, + [""]); + eq("no href outside the download path", violations.length, 0); + ok("javascript: URL dropped", "js not in " + JSON.stringify(b.names)); + ok("foreign host dropped", "foreign not in " + JSON.stringify(b.names)); + ok("traversal dropped after URL resolution", "traversal not in " + JSON.stringify(b.names)); + + // --- C. nothing usable ------------------------------------------------- + console.log("\nC. payload where no asset survives the URL check"); + const c = await run({ tag_name: "v0", assets: [{ name: "x", size: 1, browser_download_url: "javascript:1" }] }, { sumsText: "" }); + eq("no table is built", c.rows.length, 0); + eq("the pinned list stays visible", c.fallbackHidden, false); + assert.ok(/URL check/.test(c.status), "status must name the reason, not just 'could not read'"); + ok("status names the URL check as the reason"); + + // --- D. the API refuses ------------------------------------------------ + console.log("\nD. API answers 403 (rate limit)"); + const d = await run(null, { httpStatus: 403 }); + eq("the pinned list stays visible", d.fallbackHidden, false); + assert.ok(/403/.test(d.status), "status must carry the HTTP status"); + ok("status carries the HTTP status"); + + // --- E. checksums.txt unreachable -------------------------------------- + console.log("\nE. release readable, checksums.txt not"); + const e = await run(RELEASE, { sumsBroken: true }); + eq("the table still renders without hashes", e.rows.length, RELEASE.assets.length); + eq("fallback stays hidden: the table is live, just hashless", e.fallbackHidden, true); + + // --- F. the static fallback must not drift ------------------------------ + // Only meaningful against the fixture: once a newer release ships, the pinned + // table in index.html is stale by definition and the fixture is what keeps + // this check honest. + if (!live()) { + console.log("\nF. pinned fallback table in index.html"); + const pinned = [...HTML.matchAll(/([0-9a-f]{64})<\/td>/g)].map(m => m[1]); + const published = CHECKSUMS.trim().split("\n").map(l => l.split(/\s+/)[0]); + eq("pinned row count", pinned.length, 2); + pinned.forEach(h => { + assert.ok(published.includes(h), "pinned hash not in the published checksums.txt: " + h); + }); + ok("both pinned hashes match the published checksums.txt"); + assert.ok(HTML.indexOf("5.8 MiB") !== -1 && HTML.indexOf("5.4 MiB") !== -1, + "pinned sizes must match the API byte counts humanised"); + ok("pinned sizes match the API sizes humanised"); + } else { + console.log("\nF. skipped: pinned table is compared against the fixture, not the live API"); + } + + console.log("\n" + checks + " assertions, all passed"); +})().catch(err => { + console.error("\nFAILED: " + (err && err.message ? err.message : err)); + process.exit(1); +}); From d188fafc9dcc997f32f0252d707d021d737abf18 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 23:39:51 -0500 Subject: [PATCH 2/2] docs(security): write the local-token boundary and /v1/sys/opt into SECURITY.md Closes #6 as a documentation task rather than a code change, which is what the issue asked for. Two boundaries are now stated where the next reader will look, instead of sitting in an issue thread: 1. The token is a lock on the door, not on the room. GET / hands it to the page and cannot tell a browser apart from any other local process with loopback access. Spell out the two deployment cases: on the single-user workstation the token adds no exposure beyond what a same-user process already has, and on a shared or RDP host an unprivileged process that reads the token can drive an elevated --enable-collect server. Name the fix (named pipe with a SID ACL) and the issue that holds the rejected options. 2. POST /v1/sys/opt writes HKLM policy and disables DiagTrack with no flag and no body. Documented here rather than gated, because gating it would change behaviour the threat model already declares trusted. docs/security.html stops calling #6 an open design question and points at the SECURITY.md section; #6 remains as the record of the discussion. --- SECURITY.md | 33 +++++++++++++++++++++++++++++++++ docs/security.html | 8 +++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index eea9c30..d4419e7 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -40,6 +40,39 @@ protects are: **Out of scope:** physical access, kernel-level attackers, and the (by design) operator console once the token holder is trusted. +## Documented limits + +These are boundaries of the current design, not bugs, and they are written +here so they are not rediscovered as surprises. + +### The token is a lock on the door, not on the room + +`GET /` hands the token to the page that runs the dashboard, and it cannot tell +your browser apart from any other local process that asks. Anything running in +your session with loopback access can fetch the token and then call the API +with it. There is no privilege boundary between the console and a process +running as the same user, and none is claimed. + +- **Single-user workstation** (the intended deployment): a process that already + runs as you has everything the console can do, so the token adds no exposure + beyond what that process already has. +- **Shared host, terminal server, or a lower-privilege local process:** when + `--enable-collect` is set the console is elevated, so a lower-privilege + process that reads the token can ask the elevated server to export SAM/SYSTEM + or read browser stores. Do not run the console elevated with those modules + unlocked on a host where another account can reach loopback. + +If this boundary needs to be hard rather than documented, the fix is a named +pipe with an ACL restricted to the operator's SID instead of an HTTP listener; +that is tracked in issue #6. + +### `POST /v1/sys/opt` changes machine policy with no flag + +It writes registry policy and disables `DiagTrack` with no startup flag and no +request body, so anyone holding the token can change machine policy without any +of the module gates. It is documented here rather than gated, because the token +holder is trusted by design (see above). + ## Known intentional capabilities These are features, not bugs — they are the reason the tool exists: diff --git a/docs/security.html b/docs/security.html index d3b3e5f..dd2b67d 100644 --- a/docs/security.html +++ b/docs/security.html @@ -144,9 +144,11 @@

Two limits, documented rather than hidden

- Both are recorded as open design questions in - issue #6, with the options and - their trade-offs, rather than left to be discovered by whoever reads the code next. + Both are written into the repository's + SECURITY.md + under "Documented limits" rather than left to be discovered by whoever reads the code next. + The discussion, the trade-offs and the rejected options live in + issue #6, which is closed.

Intentional capabilities