diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index be6bf317..dd06ab3a 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -69,8 +69,9 @@ jobs: COREPACK_ENABLE_STRICT: '0' npm_config_ignore_scripts: 'true' YARN_ENABLE_SCRIPTS: 'false' + PACKAGE_MANAGER: ${{ inputs.package-manager }} run: | - case "${{ inputs.package-manager }}" in + case "$PACKAGE_MANAGER" in npm) npm ci --ignore-scripts ;; # pnpm v10 can fail CI on ignored native build scripts # (for example msgpackr-extract) even though this repo is Yarn-native @@ -79,7 +80,7 @@ jobs: # Yarn Berry (v4+) removed --ignore-engines; engine checking is no longer a core feature yarn) yarn install --mode=skip-build ;; bun) bun install --ignore-scripts ;; - *) echo "Unsupported package manager: ${{ inputs.package-manager }}" && exit 1 ;; + *) echo "Unsupported package manager: $PACKAGE_MANAGER" && exit 1 ;; esac - name: Run tests diff --git a/.opencode/tools/git-summary.ts b/.opencode/tools/git-summary.ts index dd11f657..b902f8e6 100644 --- a/.opencode/tools/git-summary.ts +++ b/.opencode/tools/git-summary.ts @@ -5,7 +5,24 @@ */ import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool" -import { execSync } from "child_process" +import { execFileSync } from "child_process" + +// Conservative subset of git's allowed ref-name characters. Rejects shell +// metacharacters and option-like leading `-` so a model-supplied baseBranch +// cannot inject into the shell command line built below. +const SAFE_GIT_REF = /^[A-Za-z0-9._/-]+$/ + +function isSafeRef(ref: string): boolean { + if (typeof ref !== "string" || ref.length === 0 || ref.length > 200) return false + if (!SAFE_GIT_REF.test(ref)) return false + if (ref.startsWith("-") || ref.startsWith(".") || ref.startsWith("/")) return false + if (ref.includes("..") || ref.includes("//")) return false + return true +} + +function isSafeDepth(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value > 0 && value <= 1000 +} const gitSummaryTool: ToolDefinition = tool({ description: @@ -26,19 +43,22 @@ const gitSummaryTool: ToolDefinition = tool({ }, async execute(args, context) { const cwd = context.worktree || context.directory - const depth = args.depth ?? 5 + const depth = isSafeDepth(args.depth) ? args.depth : 5 const includeDiff = args.includeDiff ?? true const baseBranch = args.baseBranch ?? "main" const result: Record = { - branch: run("git branch --show-current", cwd) || "unknown", - status: run("git status --short", cwd) || "clean", - log: run(`git log --oneline -${depth}`, cwd) || "no commits found", + branch: runArgs(["branch", "--show-current"], cwd) || "unknown", + status: runArgs(["status", "--short"], cwd) || "clean", + log: runArgs(["log", "--oneline", `-${depth}`], cwd) || "no commits found", } if (includeDiff) { - result.stagedDiff = run("git diff --cached --stat", cwd) || "" - result.branchDiff = run(`git diff ${baseBranch}...HEAD --stat`, cwd) || `unable to diff against ${baseBranch}` + result.stagedDiff = runArgs(["diff", "--cached", "--stat"], cwd) || "" + result.branchDiff = isSafeRef(baseBranch) + ? runArgs(["diff", `${baseBranch}...HEAD`, "--stat"], cwd) || + `unable to diff against ${baseBranch}` + : `unable to diff against ${baseBranch} (invalid ref)` } return JSON.stringify(result) @@ -47,9 +67,9 @@ const gitSummaryTool: ToolDefinition = tool({ export default gitSummaryTool -function run(command: string, cwd: string): string { +function runArgs(args: string[], cwd: string): string { try { - return execSync(command, { cwd, encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] }).trim() + return execFileSync("git", args, { cwd, encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] }).trim() } catch { return "" } diff --git a/scripts/lib/control-pane/server.js b/scripts/lib/control-pane/server.js index 2a1840e3..6e42533e 100644 --- a/scripts/lib/control-pane/server.js +++ b/scripts/lib/control-pane/server.js @@ -9,6 +9,43 @@ const { buildControlPaneAction } = require('./actions'); const { buildControlPaneSnapshot, resolveControlPaneConfig } = require('./state'); const { renderControlPaneHtml } = require('./ui'); +const LOOPBACK_HOSTNAMES = new Set(['127.0.0.1', 'localhost', '[::1]', '::1']); + +// Extract the hostname portion of an HTTP Host header value, stripping any +// port. Returns null when the header is missing or malformed. Used to gate +// requests against a local-only allowlist so DNS-rebinding cannot pivot a +// browser tab into the loopback control-pane API. +function parseHostHeader(value) { + if (!value || typeof value !== 'string') return null; + const trimmed = value.trim(); + if (!trimmed) return null; + const match = trimmed.match(/^(\[[^\]]+\]|[^:]+)(?::\d+)?$/); + if (!match) return null; + return match[1].toLowerCase(); +} + +function buildAllowedHostnames(configuredHost) { + const set = new Set(LOOPBACK_HOSTNAMES); + if (configuredHost) set.add(String(configuredHost).toLowerCase()); + return set; +} + +function isAllowedHostHeader(hostHeader, allowedHostnames) { + const hostname = parseHostHeader(hostHeader); + if (!hostname) return false; + return allowedHostnames.has(hostname); +} + +function isAllowedOrigin(originHeader, allowedHostnames) { + if (!originHeader || typeof originHeader !== 'string') return true; + try { + const url = new URL(originHeader); + return allowedHostnames.has(url.hostname.toLowerCase()); + } catch { + return false; + } +} + function usage() { return [ 'Usage:', @@ -159,9 +196,19 @@ function createControlPaneServer(options = {}) { env: options.env || process.env, }); const baseQuery = options.query || ''; + const allowedHostnames = buildAllowedHostnames(host); const server = http.createServer(async (req, res) => { try { + if (!isAllowedHostHeader(req.headers.host, allowedHostnames)) { + sendJson(res, 421, { ok: false, error: 'Misdirected request' }); + return; + } + if (!isAllowedOrigin(req.headers.origin, allowedHostnames)) { + sendJson(res, 403, { ok: false, error: 'Forbidden origin' }); + return; + } + const requestUrl = new URL(req.url, `http://${host}:${port || 0}`); if (req.method === 'GET' && requestUrl.pathname === '/') { @@ -280,5 +327,8 @@ module.exports = { createControlPaneServer, parseArgs, runAction, + isAllowedHostHeader, + isAllowedOrigin, + buildAllowedHostnames, usage, }; diff --git a/tests/scripts/control-pane.test.js b/tests/scripts/control-pane.test.js index 6a3fa033..1a9d74ef 100644 --- a/tests/scripts/control-pane.test.js +++ b/tests/scripts/control-pane.test.js @@ -4,6 +4,7 @@ const assert = require('assert'); const fs = require('fs'); +const http = require('http'); const os = require('os'); const path = require('path'); const { spawn, spawnSync } = require('child_process'); @@ -14,6 +15,9 @@ const { createControlPaneServer, parseArgs, runAction, + isAllowedHostHeader, + isAllowedOrigin, + buildAllowedHostnames, } = require('../../scripts/lib/control-pane/server'); const { main: runControlPaneCli, @@ -326,6 +330,92 @@ async function runTests() { } })) passed++; else failed++; + if (await test('classifies Host and Origin headers against the loopback allowlist', async () => { + const allowed = buildAllowedHostnames('127.0.0.1'); + assert.strictEqual(isAllowedHostHeader('127.0.0.1:8765', allowed), true); + assert.strictEqual(isAllowedHostHeader('localhost:8765', allowed), true); + assert.strictEqual(isAllowedHostHeader('LOCALHOST:8765', allowed), true); + assert.strictEqual(isAllowedHostHeader('[::1]:8765', allowed), true); + assert.strictEqual(isAllowedHostHeader('attacker.example.com:8765', allowed), false); + assert.strictEqual(isAllowedHostHeader('rebind.dnsbin.io', allowed), false); + assert.strictEqual(isAllowedHostHeader('', allowed), false); + assert.strictEqual(isAllowedHostHeader(undefined, allowed), false); + + // Origin is optional; absence is allowed for non-browser clients. + assert.strictEqual(isAllowedOrigin(undefined, allowed), true); + assert.strictEqual(isAllowedOrigin('', allowed), true); + assert.strictEqual(isAllowedOrigin('http://127.0.0.1:8765', allowed), true); + assert.strictEqual(isAllowedOrigin('http://localhost', allowed), true); + assert.strictEqual(isAllowedOrigin('http://attacker.example.com', allowed), false); + assert.strictEqual(isAllowedOrigin('not-a-url', allowed), false); + + // A non-default configured host should still admit loopback variants. + const lan = buildAllowedHostnames('192.168.1.10'); + assert.strictEqual(isAllowedHostHeader('192.168.1.10:8765', lan), true); + assert.strictEqual(isAllowedHostHeader('127.0.0.1:8765', lan), true); + assert.strictEqual(isAllowedHostHeader('attacker.example.com:8765', lan), false); + })) passed++; else failed++; + + if (await test('rejects requests forged with a non-loopback Host header (DNS rebinding gate)', async () => { + const app = await createControlPaneServer({ + host: '127.0.0.1', + port: 0, + repoRoot: REPO_ROOT, + allowActions: true, + }); + + await app.listen(); + try { + const address = app.server.address(); + const actualPort = address && typeof address === 'object' ? address.port : 0; + + const sendWithHeaders = (method, pathname, headers, body) => + new Promise((resolve, reject) => { + const req = http.request( + { host: '127.0.0.1', port: actualPort, method, path: pathname, headers }, + response => { + let chunks = ''; + response.on('data', chunk => { + chunks += chunk.toString('utf8'); + }); + response.on('end', () => { + resolve({ status: response.statusCode, body: chunks }); + }); + } + ); + req.on('error', reject); + if (body) req.write(body); + req.end(); + }); + + const forgedHost = await sendWithHeaders('GET', '/api/health', { Host: 'attacker.example.com:1234' }); + assert.strictEqual(forgedHost.status, 421); + assert.match(forgedHost.body, /Misdirected request/); + + const forgedActionHost = await sendWithHeaders( + 'POST', + '/api/actions/sync-knowledge', + { Host: 'attacker.example.com:1234', 'content-type': 'application/json' }, + JSON.stringify({ query: 'rebound' }) + ); + assert.strictEqual(forgedActionHost.status, 421); + + const forgedOrigin = await sendWithHeaders('GET', '/api/health', { + Host: '127.0.0.1:' + actualPort, + Origin: 'http://attacker.example.com', + }); + assert.strictEqual(forgedOrigin.status, 403); + assert.match(forgedOrigin.body, /Forbidden origin/); + + const okHost = await sendWithHeaders('GET', '/api/health', { Host: '127.0.0.1:' + actualPort }); + assert.strictEqual(okHost.status, 200); + const okBody = JSON.parse(okHost.body); + assert.strictEqual(okBody.ok, true); + } finally { + await app.close(); + } + })) passed++; else failed++; + if (await test('runAction captures success, failure, and bounded output', async () => { const repoRoot = REPO_ROOT; const success = await runAction({