From 01aa5f72749bfc8af5c3629d70bc9929c72cf6a4 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Fri, 16 Oct 2026 16:56:35 +0530 Subject: [PATCH 1/5] feat(security): add deterministic public-surface scrub Claude Code mod (part of #1497) Adds a zero-token Claude Code mod in plugins/magpie-security/hooks/ that intercepts Bash commands (git commit, gh pr create, gh pr comment) and guards public surfaces against accidental security embargo leaks (CVE IDs, vulnerability wording, security fix phrasing, and vulnerability classes). - Intercepts tool.call on Bash commands, parsing commit messages (-m, -am, -sm, -F) and PR descriptions (-t, -b, -F, --body-file). - Ingests heredocs and shell pipelines to prevent stdin bypasses. - Exempts private security tracker repositories (-s suffix convention). - Provides MAGPIE_ALLOW_EMBARGO_TERMS=1 maintainer escape hatch for legitimate post-coordinated-disclosure maintenance. - Passes clean behavioral changes through with zero token consumption. - Includes 29 unit tests covering all matching rules, command syntax, edge cases, and hook middleware integration. --- plugins/magpie-security/hooks/hooks.json | 7 + .../hooks/public-surface-scrub.test.ts | 342 ++++++++++++++++++ .../hooks/public-surface-scrub.ts | 331 +++++++++++++++++ 3 files changed, 680 insertions(+) create mode 100644 plugins/magpie-security/hooks/hooks.json create mode 100644 plugins/magpie-security/hooks/public-surface-scrub.test.ts create mode 100644 plugins/magpie-security/hooks/public-surface-scrub.ts diff --git a/plugins/magpie-security/hooks/hooks.json b/plugins/magpie-security/hooks/hooks.json new file mode 100644 index 000000000..9be5875fa --- /dev/null +++ b/plugins/magpie-security/hooks/hooks.json @@ -0,0 +1,7 @@ +{ + "$schema": "https://code.claude.com/schemas/hooks.json", + "description": "Apache Magpie — Claude Code mod for deterministic public-surface scrub against unredacted security embargo leaks.", + "modules": [ + "./public-surface-scrub.ts" + ] +} diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts new file mode 100644 index 000000000..2c676dc24 --- /dev/null +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -0,0 +1,342 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import { describe, test } from 'claude-code/testing'; +import { + evaluatePublicSurfaceCommand, + findForbiddenTerms, + isPrivateTrackerTarget, + parsePublicSurfaceCommand, + register, +} from './public-surface-scrub.ts'; + +function assert(condition: any, message?: string): void { + if (!condition) { + throw new Error(message || 'Assertion failed'); + } +} + +function assertStrictEqual(actual: any, expected: any, message?: string): void { + if (actual !== expected) { + throw new Error(message || `Expected ${JSON.stringify(expected)} but got ${JSON.stringify(actual)}`); + } +} + +function assertDeepStrictEqual(actual: any, expected: any): void { + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + throw new Error(`Expected deep equality: ${JSON.stringify(expected)} !== ${JSON.stringify(actual)}`); + } +} + +describe('Deterministic Public-Surface Scrub mod (family:security)', () => { + describe('findForbiddenTerms', () => { + test('returns empty array on neutral behavioral changes', () => { + assertDeepStrictEqual(findForbiddenTerms('Fix asset graph view leaking DAGs outside user permissions'), []); + assertDeepStrictEqual(findForbiddenTerms('Add access_key and connection_string to DEFAULT_SENSITIVE_FIELDS'), []); + assertDeepStrictEqual(findForbiddenTerms('Improve xcom value handling in extra links API'), []); + assertDeepStrictEqual(findForbiddenTerms(''), []); + assertDeepStrictEqual(findForbiddenTerms(null as any), []); + }); + + test('detects CVE references', () => { + const hits = findForbiddenTerms('Fix CVE-2026-12345 in connection test'); + assert(hits.includes('CVE reference')); + const bareHits = findForbiddenTerms('Address issue reported as cve'); + assert(bareHits.includes('CVE reference')); + }); + + test('detects vulnerability and security fix keywords (spaced and hyphenated)', () => { + assert(findForbiddenTerms('Fix critical vulnerability in parsing logic').includes('vulnerability')); + assert(findForbiddenTerms('Security fix for webserver endpoints').includes('security fix')); + assert(findForbiddenTerms('Apply security-fix for session timeout').includes('security fix')); + assert(findForbiddenTerms('Security patch for query builder').includes('security fix')); + assert(findForbiddenTerms('Apply critical security-patch').includes('security fix')); + assert(findForbiddenTerms('Contact security@apache.org for details').includes('security mailing list')); + }); + + test('detects vulnerability classes with whitespace and hyphens', () => { + assert(findForbiddenTerms('Prevent SSRF in HTTP provider').includes('SSRF')); + assert(findForbiddenTerms('Mitigate stored XSS in log view').includes('XSS')); + assert(findForbiddenTerms('Add CSRF token validation').includes('CSRF')); + assert(findForbiddenTerms('Prevent SQL injection in search filter').includes('SQL injection')); + assert(findForbiddenTerms('Mitigate SQL-injection in query filter').includes('SQL injection')); + assert(findForbiddenTerms('Fix path traversal in file download').includes('path traversal')); + assert(findForbiddenTerms('Prevent path-traversal in file handler').includes('path traversal')); + assert(findForbiddenTerms('Prevent remote code execution in worker').includes('remote code execution')); + assert(findForbiddenTerms('Close RCE vector in serializer').includes('remote code execution')); + assert(findForbiddenTerms('Fix remote-code-execution vulnerability').includes('remote code execution')); + assert(findForbiddenTerms('Fix privilege escalation in role mapper').includes('privilege escalation')); + assert(findForbiddenTerms('Prevent privilege-escalation bug').includes('privilege escalation')); + assert(findForbiddenTerms('Fix auth bypass in REST API').includes('auth bypass')); + assert(findForbiddenTerms('Mitigate auth-bypass vulnerability').includes('auth bypass')); + }); + + test('returns multiple matched terms uniquely', () => { + const terms = findForbiddenTerms('Fix CVE-2026-9999: critical SSRF vulnerability allowing auth-bypass'); + assert(terms.includes('CVE reference')); + assert(terms.includes('SSRF')); + assert(terms.includes('vulnerability')); + assert(terms.includes('auth bypass')); + }); + }); + + describe('isPrivateTrackerTarget', () => { + test('identifies ASF security tracker repo format with -s suffix', () => { + assertStrictEqual(isPrivateTrackerTarget('gh pr comment 123 --repo airflow-s/airflow-s --body "Details"'), true); + assertStrictEqual(isPrivateTrackerTarget('gh pr create -R myorg/kafka-s -b "Fix"'), true); + assertStrictEqual(isPrivateTrackerTarget('gh issue view 5 -R org/security-tracker'), true); + assertStrictEqual(isPrivateTrackerTarget('gh pr view 10 -R org/repo-private'), true); + }); + + test('returns false for public upstream repositories', () => { + assertStrictEqual(isPrivateTrackerTarget('gh pr create --repo apache/airflow --body "Details"'), false); + assertStrictEqual(isPrivateTrackerTarget('gh pr comment 123 -R apache/kafka -b "Fix"'), false); + assertStrictEqual(isPrivateTrackerTarget('git commit -m "Fix"'), false); + }); + }); + + describe('parsePublicSurfaceCommand', () => { + test('identifies clean git commit with -m', () => { + const parsed = parsePublicSurfaceCommand('git commit -m "Fix asset graph view"'); + assertStrictEqual(parsed.isPublicSurfaceCommand, true); + assertStrictEqual(parsed.commandType, 'git commit'); + assertDeepStrictEqual(parsed.extractedTexts, ['Fix asset graph view']); + assertDeepStrictEqual(parsed.referencedFiles, []); + }); + + test('identifies git commit with combined short flags like -am and -sm', () => { + const amParsed = parsePublicSurfaceCommand('git commit -am "Fix asset graph view"'); + assertStrictEqual(amParsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(amParsed.extractedTexts, ['Fix asset graph view']); + + const smParsed = parsePublicSurfaceCommand('git commit -sm "Fix asset graph view"'); + assertStrictEqual(smParsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(smParsed.extractedTexts, ['Fix asset graph view']); + + const samParsed = parsePublicSurfaceCommand('git commit -sam "Fix asset graph view"'); + assertStrictEqual(samParsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(samParsed.extractedTexts, ['Fix asset graph view']); + }); + + test('identifies git -C commit with options that take arguments', () => { + const parsed = parsePublicSurfaceCommand('git -C /path/to/upstream commit -s -m "Fix asset graph view"'); + assertStrictEqual(parsed.isPublicSurfaceCommand, true); + assertStrictEqual(parsed.commandType, 'git commit'); + assertDeepStrictEqual(parsed.extractedTexts, ['Fix asset graph view']); + }); + + test('identifies git commit with multiple -m flags and flags in between', () => { + const parsed = parsePublicSurfaceCommand('git commit -s -m "Header line" -m "Body description"'); + assertStrictEqual(parsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(parsed.extractedTexts, ['Header line', 'Body description']); + }); + + test('identifies git commit with -F message file', () => { + const parsed = parsePublicSurfaceCommand('git commit -F /tmp/commit-msg.txt'); + assertStrictEqual(parsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(parsed.referencedFiles, ['/tmp/commit-msg.txt']); + }); + + test('detects heredoc and shell pipeline usage', () => { + const heredocParsed = parsePublicSurfaceCommand("git commit -F - << 'EOF'\nFix CVE-2026-1234\nEOF"); + assertStrictEqual(heredocParsed.isPublicSurfaceCommand, true); + assertStrictEqual(heredocParsed.hasComplexShellInput, true); + + const pipeParsed = parsePublicSurfaceCommand('echo "msg" | gh pr create --title "Title" --body-file -'); + assertStrictEqual(pipeParsed.isPublicSurfaceCommand, true); + assertStrictEqual(pipeParsed.hasComplexShellInput, true); + }); + + test('identifies gh pr create with title, body, and body-file', () => { + const parsed = parsePublicSurfaceCommand('gh pr create --title "Neutral title" --body "Neutral body"'); + assertStrictEqual(parsed.isPublicSurfaceCommand, true); + assertStrictEqual(parsed.commandType, 'gh pr create'); + assert(parsed.extractedTexts.includes('Neutral title')); + assert(parsed.extractedTexts.includes('Neutral body')); + + const fileParsed = parsePublicSurfaceCommand('gh pr create -t "Title" --body-file /tmp/pr.md'); + assertStrictEqual(fileParsed.isPublicSurfaceCommand, true); + assertDeepStrictEqual(fileParsed.referencedFiles, ['/tmp/pr.md']); + }); + + test('skips public surface parsing when target is a private tracker repo', () => { + const parsed = parsePublicSurfaceCommand('gh pr comment 123 --repo airflow-s/airflow-s --body "Fixes CVE-2026-1"'); + assertStrictEqual(parsed.isPrivateTrackerTarget, true); + assertStrictEqual(parsed.isPublicSurfaceCommand, false); + }); + + test('ignores non-public surface commands', () => { + assertStrictEqual(parsePublicSurfaceCommand('pytest tests/').isPublicSurfaceCommand, false); + assertStrictEqual(parsePublicSurfaceCommand('git status').isPublicSurfaceCommand, false); + assertStrictEqual(parsePublicSurfaceCommand('git diff HEAD~1').isPublicSurfaceCommand, false); + assertStrictEqual(parsePublicSurfaceCommand('uv run ruff check').isPublicSurfaceCommand, false); + assertStrictEqual(parsePublicSurfaceCommand('').isPublicSurfaceCommand, false); + }); + }); + + describe('evaluatePublicSurfaceCommand', () => { + test('allows clean neutral commit', async () => { + const evaluation = await evaluatePublicSurfaceCommand('git commit -m "Fix asset graph view"'); + assertStrictEqual(evaluation.blocked, false); + assertDeepStrictEqual(evaluation.matchedTerms, []); + }); + + test('blocks git commit -am containing CVE reference', async () => { + const evaluation = await evaluatePublicSurfaceCommand('git commit -am "Fix CVE-2026-5555 in connection test"'); + assertStrictEqual(evaluation.blocked, true); + assert(evaluation.matchedTerms.includes('CVE reference')); + }); + + test('blocks git -C commit containing hyphenated vulnerability class', async () => { + const evaluation = await evaluatePublicSurfaceCommand('git -C /path/to/upstream commit -m "Fix SQL-injection in query"'); + assertStrictEqual(evaluation.blocked, true); + assert(evaluation.matchedTerms.includes('SQL injection')); + assert(evaluation.reason?.includes('Apache Magpie Security Guard')); + assert(evaluation.reason?.includes('Confidentiality Rule')); + }); + + test('blocks commit reading message file that contains forbidden terms', async () => { + const mockFiles: Record = { + '/tmp/msg.txt': 'Stop deserializing XCom values\n\nFixes arbitrary SQL injection in worker.', + }; + const evaluation = await evaluatePublicSurfaceCommand('git commit -F /tmp/msg.txt', { + readFile: async (p) => mockFiles[p] || '', + }); + + assertStrictEqual(evaluation.blocked, true); + assert(evaluation.matchedTerms.includes('SQL injection')); + }); + + test('blocks heredoc commit containing forbidden terms', async () => { + const heredocCmd = "git commit -F - << 'EOF'\nFix CVE-2026-1111: Critical SSRF vulnerability\nEOF"; + const evaluation = await evaluatePublicSurfaceCommand(heredocCmd); + assertStrictEqual(evaluation.blocked, true); + assert(evaluation.matchedTerms.includes('CVE reference')); + assert(evaluation.matchedTerms.includes('SSRF')); + assert(evaluation.matchedTerms.includes('vulnerability')); + }); + + test('blocks piped command line containing forbidden terms', async () => { + const pipeCmd = 'echo "Fixes critical SSRF vulnerability" | gh pr create --title "Fix" --body-file -'; + const evaluation = await evaluatePublicSurfaceCommand(pipeCmd); + assertStrictEqual(evaluation.blocked, true); + assert(evaluation.matchedTerms.includes('SSRF')); + assert(evaluation.matchedTerms.includes('vulnerability')); + }); + + test('allows private tracker commands even when mentioning CVEs and vulnerability classes', async () => { + const trackerCmd = 'gh pr comment 45 --repo apache/airflow-s --body "Confirmed SSRF fix for CVE-2026-1234"'; + const evaluation = await evaluatePublicSurfaceCommand(trackerCmd); + assertStrictEqual(evaluation.blocked, false); + }); + + test('honors maintainer escape hatch via env var', async () => { + const leakingCmd = 'git commit -m "Backport CVE-2026-1234 release notes"'; + const evaluation = await evaluatePublicSurfaceCommand(leakingCmd, { + env: { MAGPIE_ALLOW_EMBARGO_TERMS: '1' }, + }); + assertStrictEqual(evaluation.blocked, false); + }); + + test('honors maintainer escape hatch via inline flag', async () => { + const leakingCmd = 'MAGPIE_ALLOW_EMBARGO_TERMS=1 git commit -m "Backport CVE-2026-1234"'; + const evaluation = await evaluatePublicSurfaceCommand(leakingCmd); + assertStrictEqual(evaluation.blocked, false); + }); + + test('passes non-target commands through immediately', async () => { + const evaluation = await evaluatePublicSurfaceCommand('git log -n 5 --grep "CVE-"'); + assertStrictEqual(evaluation.blocked, false); + }); + }); + + describe('Claude Code hook integration (register)', () => { + test('allows clean Bash tool call through via next()', async () => { + let registeredHook: any = null; + const on = (event: string, handler: any) => { + if (event === 'tool.call') { + registeredHook = handler; + } + }; + + register(on); + assert(registeredHook, 'tool.call hook must be registered'); + + let nextCalled = false; + const mockEvent = { tool: 'Bash', input: { command: 'git commit -m "Fix asset graph view"' } }; + const next = (e: any) => { + nextCalled = true; + return { success: true, event: e }; + }; + + const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); + assertStrictEqual(nextCalled, true); + assertStrictEqual(result.success, true); + }); + + test('intercepts and blocks leaking Bash tool call with combined -am flag returning error', async () => { + let registeredHook: any = null; + const on = (event: string, handler: any) => { + if (event === 'tool.call') { + registeredHook = handler; + } + }; + + register(on); + + let nextCalled = false; + const mockEvent = { + tool: 'Bash', + input: { command: 'git -C /upstream commit -am "Fix CVE-2026-1111: SSRF vulnerability"' }, + }; + const next = () => { + nextCalled = true; + }; + + const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); + assertStrictEqual(nextCalled, false, 'next() must NOT be called on blocked commands'); + assert(result.error, 'Error explanation must be returned'); + assert(result.error.includes('Apache Magpie Security Guard')); + assert(result.error.includes('CVE reference')); + assert(result.error.includes('SSRF')); + }); + + test('passes non-Bash tools through to next() unchanged', async () => { + let registeredHook: any = null; + const on = (event: string, handler: any) => { + if (event === 'tool.call') { + registeredHook = handler; + } + }; + + register(on); + + let nextCalled = false; + const mockEvent = { tool: 'ReadUrl', input: { url: 'https://cve.org/CVE-2026-1234' } }; + const next = (e: any) => { + nextCalled = true; + return { forwarded: e }; + }; + + const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); + assertStrictEqual(nextCalled, true); + assertStrictEqual(result.forwarded.tool, 'ReadUrl'); + }); + }); +}); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts new file mode 100644 index 000000000..1149023fb --- /dev/null +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +/** + * Single forbidden-term regexes for public surfaces, aligned with + * plugins/magpie-security/skills/issue-fix/implementation-plan.md:35-43. + * Supports spaces and hyphens in compound vulnerability terms (e.g. SQL-injection). + */ +export const FORBIDDEN_TERM_RULES: ReadonlyArray<{ name: string; pattern: RegExp }> = [ + { name: 'CVE reference', pattern: /\bCVE(?:-\d{4}-\d+)?\b/i }, + { name: 'vulnerability', pattern: /\bvulnerabilit(?:y|ies)\b/i }, + { name: 'security fix', pattern: /\bsecurity[\s-]+(?:fix|patch|advisory)\b/i }, + { name: 'security mailing list', pattern: /\bsecurity@/i }, + { name: 'SSRF', pattern: /\bSSRF\b/i }, + { name: 'XSS', pattern: /\bXSS\b/i }, + { name: 'CSRF', pattern: /\bCSRF\b/i }, + { name: 'SQL injection', pattern: /\bSQL[\s-]+injection\b/i }, + { name: 'path traversal', pattern: /\bpath[\s-]+traversal\b/i }, + { name: 'remote code execution', pattern: /\b(?:remote[\s-]+code[\s-]+execution|RCE)\b/i }, + { name: 'privilege escalation', pattern: /\bprivilege[\s-]+escalation\b/i }, + { name: 'auth bypass', pattern: /\bauth(?:orization|entication)?[\s-]+bypass\b/i }, +]; + +/** + * Finds all unique forbidden embargo terms that appear in the candidate text. + */ +export function findForbiddenTerms(text: string): string[] { + if (!text || typeof text !== 'string') { + return []; + } + + const matched: string[] = []; + for (const rule of FORBIDDEN_TERM_RULES) { + if (rule.pattern.test(text)) { + matched.push(rule.name); + } + } + + return matched; +} + +export interface ExtractedPublicSurface { + isPublicSurfaceCommand: boolean; + isPrivateTrackerTarget: boolean; + commandType: string; + extractedTexts: string[]; + referencedFiles: string[]; + hasComplexShellInput: boolean; +} + +/** + * Checks whether an explicit --repo or -R argument points to a private security tracker. + * ASF security trackers use the `-s/-s` convention or contain `-s` / `security` / `private`. + */ +export function isPrivateTrackerTarget(commandLine: string): boolean { + if (!commandLine || typeof commandLine !== 'string') { + return false; + } + + const repoMatch = commandLine.match(/(?:--repo|-R)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/i); + if (!repoMatch) { + return false; + } + + const repo = (repoMatch[1] ?? repoMatch[2] ?? repoMatch[3] ?? '').toLowerCase(); + if (!repo) { + return false; + } + + // Suffix '-s' (e.g. airflow-s, airflow-s/airflow-s), or names containing private/security tracker + return ( + repo.endsWith('-s') || + repo.includes('-s/') || + repo.includes('/private') || + repo.includes('-private') || + repo.includes('security-tracker') + ); +} + +/** + * Inspects a Bash command line to determine if it is a public-surface git/gh command, + * extracting any inline commit/PR messages and referenced message files. + * Handles combined short flags (e.g. -am, -sm) and complex shell inputs (heredocs, pipes). + */ +export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicSurface { + const result: ExtractedPublicSurface = { + isPublicSurfaceCommand: false, + isPrivateTrackerTarget: false, + commandType: '', + extractedTexts: [], + referencedFiles: [], + hasComplexShellInput: false, + }; + + if (!commandLine || typeof commandLine !== 'string') { + return result; + } + + const trimmed = commandLine.trim(); + + // Check if this targets a private tracker (e.g. gh pr comment --repo airflow-s/airflow-s) + if (isPrivateTrackerTarget(trimmed)) { + result.isPrivateTrackerTarget = true; + return result; + } + + // Detect heredocs (<<), shell pipelines (|), or process substitution (<() + const hasComplexShellInput = /<<|\||<\(/.test(trimmed); + result.hasComplexShellInput = hasComplexShellInput; + + // Match git commit commands, including git -C commit, git -c = commit, git commit -am, etc. + if (/(?:^|[;&|]\s*)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) { + result.isPublicSurfaceCommand = true; + result.commandType = 'git commit'; + + // Extract -m, -am, -sm, -sam, or --message="..." + const mMatches = trimmed.matchAll(/(?:-[a-zA-Z]*m|--message)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + for (const m of mMatches) { + const text = m[1] ?? m[2] ?? m[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + // Extract -F or --file= + const fMatches = trimmed.matchAll(/(?:-F|--file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + for (const f of fMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file && file !== '-') { + result.referencedFiles.push(file); + } + } + + return result; + } + + // Match gh pr create commands + if (/(?:^|[;&|]\s*)gh\s+pr\s+create(?:\s|$)/i.test(trimmed)) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr create'; + + // Title: -t "..." or --title="..." + const titleMatches = trimmed.matchAll(/(?:-t|--title)(?:=|\s+)(?:"([^"]*)"|'([^']*)'|([^\s;]+))/g); + for (const t of titleMatches) { + const text = t[1] ?? t[2] ?? t[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + // Body: -b "..." or --body="..." + const bodyMatches = trimmed.matchAll(/(?:-b|--body)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + for (const b of bodyMatches) { + const text = b[1] ?? b[2] ?? b[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + // Body file: -F or --body-file= + const fileMatches = trimmed.matchAll(/(?:-F|--body-file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + for (const f of fileMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file && file !== '-') { + result.referencedFiles.push(file); + } + } + + return result; + } + + // Match gh pr comment commands + if (/(?:^|[;&|]\s*)gh\s+pr\s+comment(?:\s|$)/i.test(trimmed)) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr comment'; + + // Body: -b "..." or --body="..." + const bodyMatches = trimmed.matchAll(/(?:-b|--body)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + for (const b of bodyMatches) { + const text = b[1] ?? b[2] ?? b[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + // Body file: -F or --body-file= + const fileMatches = trimmed.matchAll(/(?:-F|--body-file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + for (const f of fileMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file && file !== '-') { + result.referencedFiles.push(file); + } + } + + return result; + } + + return result; +} + +export type FileReader = (filePath: string) => Promise; + +export interface EvaluationOptions { + readFile?: FileReader; + env?: Record; +} + +/** + * Evaluates whether a public-surface command violates embargo terms. + * Returns null if allowed, or an actionable blocking error message if violated. + */ +export async function evaluatePublicSurfaceCommand( + commandLine: string, + options?: EvaluationOptions +): Promise<{ blocked: boolean; matchedTerms: string[]; reason?: string }> { + // Check for explicit maintainer escape hatch (e.g. post-disclosure release notes or backport PRs) + const envVal = options?.env?.MAGPIE_ALLOW_EMBARGO_TERMS; + if ( + envVal === '1' || + /(?:^|\s)MAGPIE_ALLOW_EMBARGO_TERMS=1(?:\s|$)/.test(commandLine) || + commandLine.includes('--allow-embargo-terms') + ) { + return { blocked: false, matchedTerms: [] }; + } + + const parsed = parsePublicSurfaceCommand(commandLine); + if (!parsed.isPublicSurfaceCommand || parsed.isPrivateTrackerTarget) { + return { blocked: false, matchedTerms: [] }; + } + + const allTexts: string[] = [...parsed.extractedTexts]; + + // If heredocs, pipes, or process substitution are used, scan the whole raw command line as well + if (parsed.hasComplexShellInput) { + allTexts.push(commandLine); + } + + // If external message files are referenced, attempt to read their contents + if (options?.readFile && parsed.referencedFiles.length > 0) { + for (const file of parsed.referencedFiles) { + try { + const content = await options.readFile(file); + if (content) { + allTexts.push(content); + } + } catch { + // Safe skip on unreadable file; pure regex inspection continues + } + } + } + + const combinedText = allTexts.join('\n'); + const matchedTerms = findForbiddenTerms(combinedText); + + if (matchedTerms.length === 0) { + return { blocked: false, matchedTerms: [] }; + } + + const uniqueTerms = Array.from(new Set(matchedTerms)); + const termList = uniqueTerms.map((t) => `\`${t}\``).join(', '); + + const reason = [ + `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}).`, + `Found forbidden security disclosure term(s): ${termList}.`, + '', + 'Confidentiality Rule (AGENTS.md & docs/security/fixing-security-issues.md):', + 'Public commits, PR descriptions, and PR comments on upstream repositories must describe the behavioral change in neutral terms (e.g. "Fix asset graph view leaking DAGs outside permissions").', + 'They must NEVER mention CVE IDs, "security fix", "vulnerability", or vulnerability classes (SSRF, XSS, etc.) before the coordinated disclosure advisory has shipped.', + '', + 'If this is a legitimate post-disclosure change, set MAGPIE_ALLOW_EMBARGO_TERMS=1 to proceed.', + ].join('\n'); + + return { + blocked: true, + matchedTerms: uniqueTerms, + reason, + }; +} + +/** + * Event hook registration entry point for Claude Code. + */ +export function register(on: any): void { + // Hook tool.call to guard Bash executions on public surfaces + on('tool.call', async ($: any, e: any, next: any) => { + // Check if the tool is Bash and has a command parameter + const toolName = (e?.tool || '').toLowerCase(); + const command = e?.input?.command || e?.params?.command; + + if (toolName === 'bash' && typeof command === 'string') { + const evaluation = await evaluatePublicSurfaceCommand(command, { + readFile: async (filePath: string) => { + try { + return await $.fs.read(filePath); + } catch { + return ''; + } + }, + }); + + if (evaluation.blocked && evaluation.reason) { + // Return blocking error response to halt execution and educate the user/agent + return { + error: evaluation.reason, + }; + } + } + + if (typeof next === 'function') { + return next(e); + } + }); +} + +export default register; From a0d9efdaf7b00f5757fe7181b065c0b10c0a1fa8 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Sat, 17 Oct 2026 10:33:12 +0530 Subject: [PATCH 2/5] fix(security): address maintainer review on deterministic public-surface scrub mod - Limit escape hatch strictly to options.env set by human, removing in-command bypasses. - Scope checks to upstream_repo from project.md; evaluate private tracker exemption per subcommand. - Split multi-command pipelines on shell separators (including newlines) and accept attached short flags. - Fail closed on unreadable message files and indeterminate dynamic input. - Resolve file paths relative to git -C directory without node:path. - Expand test suite to 47 unit tests covering all review shapes. --- .../hooks/public-surface-scrub.test.ts | 599 ++++++++++++---- .../hooks/public-surface-scrub.ts | 658 ++++++++++++++---- 2 files changed, 1020 insertions(+), 237 deletions(-) diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index 2c676dc24..d893a042d 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -21,9 +21,12 @@ import { describe, test } from 'claude-code/testing'; import { evaluatePublicSurfaceCommand, findForbiddenTerms, - isPrivateTrackerTarget, - parsePublicSurfaceCommand, + parseProjectManifest, + parseSubcommand, register, + resolvePath, + resolveProjectConfig, + splitShellCommands, } from './public-surface-scrub.ts'; function assert(condition: any, message?: string): void { @@ -67,6 +70,7 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { assert(findForbiddenTerms('Apply security-fix for session timeout').includes('security fix')); assert(findForbiddenTerms('Security patch for query builder').includes('security fix')); assert(findForbiddenTerms('Apply critical security-patch').includes('security fix')); + assert(findForbiddenTerms('Security advisory update').includes('security fix')); assert(findForbiddenTerms('Contact security@apache.org for details').includes('security mailing list')); }); @@ -96,173 +100,503 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { }); }); - describe('isPrivateTrackerTarget', () => { - test('identifies ASF security tracker repo format with -s suffix', () => { - assertStrictEqual(isPrivateTrackerTarget('gh pr comment 123 --repo airflow-s/airflow-s --body "Details"'), true); - assertStrictEqual(isPrivateTrackerTarget('gh pr create -R myorg/kafka-s -b "Fix"'), true); - assertStrictEqual(isPrivateTrackerTarget('gh issue view 5 -R org/security-tracker'), true); - assertStrictEqual(isPrivateTrackerTarget('gh pr view 10 -R org/repo-private'), true); + describe('parseProjectManifest & resolveProjectConfig', () => { + test('extracts tracker_repo and upstream_repo from markdown tables', () => { + const sample = ` +# Project Manifest +| Key | Value | Purpose | +|---|---|---| +| \`tracker_repo\` | \`airflow-s/airflow-s\` | Private security tracker | +| \`upstream_repo\` | \`apache/airflow\` | Public codebase where fixes land | +`; + const parsed = parseProjectManifest(sample); + assertStrictEqual(parsed.trackerRepo, 'airflow-s/airflow-s'); + assertStrictEqual(parsed.upstreamRepo, 'apache/airflow'); }); - test('returns false for public upstream repositories', () => { - assertStrictEqual(isPrivateTrackerTarget('gh pr create --repo apache/airflow --body "Details"'), false); - assertStrictEqual(isPrivateTrackerTarget('gh pr comment 123 -R apache/kafka -b "Fix"'), false); - assertStrictEqual(isPrivateTrackerTarget('git commit -m "Fix"'), false); + test('extracts tracker_repo and upstream_repo from yaml format', () => { + const sample = ` +tracker_repo: kafka-s/kafka-s +upstream_repo: apache/kafka +`; + const parsed = parseProjectManifest(sample); + assertStrictEqual(parsed.trackerRepo, 'kafka-s/kafka-s'); + assertStrictEqual(parsed.upstreamRepo, 'apache/kafka'); }); - }); - describe('parsePublicSurfaceCommand', () => { - test('identifies clean git commit with -m', () => { - const parsed = parsePublicSurfaceCommand('git commit -m "Fix asset graph view"'); - assertStrictEqual(parsed.isPublicSurfaceCommand, true); - assertStrictEqual(parsed.commandType, 'git commit'); - assertDeepStrictEqual(parsed.extractedTexts, ['Fix asset graph view']); - assertDeepStrictEqual(parsed.referencedFiles, []); + test('ignores TODO placeholders in project.md template', () => { + const sample = ` +| tracker_repo | TODO: e.g. foo-s/foo-s | +| upstream_repo | TODO: e.g. apache/foo | +`; + const parsed = parseProjectManifest(sample); + assertStrictEqual(parsed.trackerRepo, undefined); + assertStrictEqual(parsed.upstreamRepo, undefined); }); - test('identifies git commit with combined short flags like -am and -sm', () => { - const amParsed = parsePublicSurfaceCommand('git commit -am "Fix asset graph view"'); - assertStrictEqual(amParsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(amParsed.extractedTexts, ['Fix asset graph view']); + test('ignores "not used" descriptions and extracts clean slugs with annotations', () => { + const sample = ` +| tracker_repo | not used — Apache Magpie has no private security tracker; reports follow the ASF default process via security@apache.org | +| upstream_repo | apache/magpie (derived from the upstream git remote) | +`; + const parsed = parseProjectManifest(sample); + assertStrictEqual(parsed.trackerRepo, undefined); + assertStrictEqual(parsed.upstreamRepo, 'apache/magpie'); + }); - const smParsed = parsePublicSurfaceCommand('git commit -sm "Fix asset graph view"'); - assertStrictEqual(smParsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(smParsed.extractedTexts, ['Fix asset graph view']); + test('resolveProjectConfig probes in precedence order', async () => { + const mockFiles: Record = { + '.apache-magpie-overrides/project.md': '| tracker_repo | overrides-s/tracker |\n| upstream_repo | apache/overrides |', + }; - const samParsed = parsePublicSurfaceCommand('git commit -sam "Fix asset graph view"'); - assertStrictEqual(samParsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(samParsed.extractedTexts, ['Fix asset graph view']); + const resolved = await resolveProjectConfig(async (p) => mockFiles[p]); + assertStrictEqual(resolved.trackerRepo, 'overrides-s/tracker'); + assertStrictEqual(resolved.upstreamRepo, 'apache/overrides'); + assertStrictEqual(resolved.isTrackerCheckout, true); }); + }); - test('identifies git -C commit with options that take arguments', () => { - const parsed = parsePublicSurfaceCommand('git -C /path/to/upstream commit -s -m "Fix asset graph view"'); + describe('splitShellCommands', () => { + test('splits on semicolons, conditional operators, pipes, and newlines', () => { + const raw = 'git status; git commit -m "Clean"\ngh pr create -t "Title" && git tag v1.0 || exit 1'; + const parts = splitShellCommands(raw); + assertDeepStrictEqual(parts, [ + 'git status', + 'git commit -m "Clean"', + 'gh pr create -t "Title"', + 'git tag v1.0', + 'exit 1', + ]); + }); + + test('preserves semicolons and newlines inside quoted strings', () => { + const raw = 'git commit -m "Header line; second part\nThird line" ; git push'; + const parts = splitShellCommands(raw); + assertStrictEqual(parts.length, 2); + assertStrictEqual(parts[0], 'git commit -m "Header line; second part\nThird line"'); + assertStrictEqual(parts[1], 'git push'); + }); + }); + + describe('resolvePath', () => { + test('resolves relative path against git -C directory without node:path', () => { + assertStrictEqual(resolvePath('/path/to/upstream', 'msg.txt'), '/path/to/upstream/msg.txt'); + assertStrictEqual(resolvePath('/path/to/upstream/', 'msg.txt'), '/path/to/upstream/msg.txt'); + assertStrictEqual(resolvePath('/path/to/upstream', './msg.txt'), '/path/to/upstream/msg.txt'); + assertStrictEqual(resolvePath('/path/to/upstream', '"msg.txt"'), '/path/to/upstream/msg.txt'); + assertStrictEqual(resolvePath('/path/to/upstream', '/abs/msg.txt'), '/abs/msg.txt'); + assertStrictEqual(resolvePath(undefined, 'msg.txt'), 'msg.txt'); + }); + }); + + describe('parseSubcommand', () => { + test('identifies git commit with -m and combined flags', () => { + const parsed = parseSubcommand('git commit -m "Fix asset graph view"'); assertStrictEqual(parsed.isPublicSurfaceCommand, true); assertStrictEqual(parsed.commandType, 'git commit'); assertDeepStrictEqual(parsed.extractedTexts, ['Fix asset graph view']); + + const am = parseSubcommand('git commit -am "Fix view"'); + assertDeepStrictEqual(am.extractedTexts, ['Fix view']); + + const sam = parseSubcommand('git commit -sam "Signed and added"'); + assertDeepStrictEqual(sam.extractedTexts, ['Signed and added']); }); - test('identifies git commit with multiple -m flags and flags in between', () => { - const parsed = parsePublicSurfaceCommand('git commit -s -m "Header line" -m "Body description"'); - assertStrictEqual(parsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(parsed.extractedTexts, ['Header line', 'Body description']); + test('identifies attached short options without whitespace (-m"...", -m\'...\', -mval, -Fpath)', () => { + const attachedM = parseSubcommand('git commit -m"Fix view"'); + assertDeepStrictEqual(attachedM.extractedTexts, ['Fix view']); + + const attachedSingleM = parseSubcommand("git commit -m'Fix view'"); + assertDeepStrictEqual(attachedSingleM.extractedTexts, ['Fix view']); + + const attachedBareM = parseSubcommand('git commit -mFixView'); + assertDeepStrictEqual(attachedBareM.extractedTexts, ['FixView']); + + const attachedF = parseSubcommand('git commit -Fmsg.txt'); + assertDeepStrictEqual(attachedF.referencedFiles, ['msg.txt']); + + const attachedT = parseSubcommand('gh pr create -t"Title" -b"Body"'); + assert(attachedT.extractedTexts.includes('Title')); + assert(attachedT.extractedTexts.includes('Body')); }); - test('identifies git commit with -F message file', () => { - const parsed = parsePublicSurfaceCommand('git commit -F /tmp/commit-msg.txt'); - assertStrictEqual(parsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(parsed.referencedFiles, ['/tmp/commit-msg.txt']); + test('identifies git tag with annotated message or file', () => { + const tagM = parseSubcommand('git tag -a v1.0.0 -m "Release v1.0.0"'); + assertStrictEqual(tagM.isPublicSurfaceCommand, true); + assertStrictEqual(tagM.commandType, 'git tag'); + assertDeepStrictEqual(tagM.extractedTexts, ['Release v1.0.0']); + + const tagF = parseSubcommand('git tag -F tag-notes.txt v1.0.0'); + assertStrictEqual(tagF.isPublicSurfaceCommand, true); + assertDeepStrictEqual(tagF.referencedFiles, ['tag-notes.txt']); + + // Lightweight tag without message is not a public surface publishing command + const bareTag = parseSubcommand('git tag v1.0.0'); + assertStrictEqual(bareTag.isPublicSurfaceCommand, false); }); - test('detects heredoc and shell pipeline usage', () => { - const heredocParsed = parsePublicSurfaceCommand("git commit -F - << 'EOF'\nFix CVE-2026-1234\nEOF"); - assertStrictEqual(heredocParsed.isPublicSurfaceCommand, true); - assertStrictEqual(heredocParsed.hasComplexShellInput, true); + test('identifies gh pr edit and gh pr review', () => { + const edit = parseSubcommand('gh pr edit 123 --title "New Title" --body "New Body"'); + assertStrictEqual(edit.isPublicSurfaceCommand, true); + assertStrictEqual(edit.commandType, 'gh pr edit'); + assert(edit.extractedTexts.includes('New Title')); + assert(edit.extractedTexts.includes('New Body')); + + const review = parseSubcommand('gh pr review 123 --body "Approved changes"'); + assertStrictEqual(review.isPublicSurfaceCommand, true); + assertStrictEqual(review.commandType, 'gh pr review'); + assertDeepStrictEqual(review.extractedTexts, ['Approved changes']); + }); - const pipeParsed = parsePublicSurfaceCommand('echo "msg" | gh pr create --title "Title" --body-file -'); - assertStrictEqual(pipeParsed.isPublicSurfaceCommand, true); - assertStrictEqual(pipeParsed.hasComplexShellInput, true); + test('identifies gh issue comment', () => { + const comment = parseSubcommand('gh issue comment 456 --body "Under investigation"'); + assertStrictEqual(comment.isPublicSurfaceCommand, true); + assertStrictEqual(comment.commandType, 'gh pr comment'); + assertDeepStrictEqual(comment.extractedTexts, ['Under investigation']); }); - test('identifies gh pr create with title, body, and body-file', () => { - const parsed = parsePublicSurfaceCommand('gh pr create --title "Neutral title" --body "Neutral body"'); - assertStrictEqual(parsed.isPublicSurfaceCommand, true); - assertStrictEqual(parsed.commandType, 'gh pr create'); - assert(parsed.extractedTexts.includes('Neutral title')); - assert(parsed.extractedTexts.includes('Neutral body')); + test('identifies gh api comment creation with -f body=... and quoted key-value pairs', () => { + const apiCmd = parseSubcommand('gh api repos/apache/airflow/issues/1/comments -f body="Fixed problem"'); + assertStrictEqual(apiCmd.isPublicSurfaceCommand, true); + assertStrictEqual(apiCmd.commandType, 'gh api'); + assertStrictEqual(apiCmd.targetRepo, 'apache/airflow'); + assertDeepStrictEqual(apiCmd.extractedTexts, ['Fixed problem']); + + const apiQuotedPair = parseSubcommand('gh api repos/apache/airflow/issues/1/comments -f "body=Fixed problem"'); + assertDeepStrictEqual(apiQuotedPair.extractedTexts, ['Fixed problem']); - const fileParsed = parsePublicSurfaceCommand('gh pr create -t "Title" --body-file /tmp/pr.md'); - assertStrictEqual(fileParsed.isPublicSurfaceCommand, true); - assertDeepStrictEqual(fileParsed.referencedFiles, ['/tmp/pr.md']); + const apiField = parseSubcommand('gh api repos/apache/airflow/issues/1/comments --field "body=Fixed problem"'); + assertDeepStrictEqual(apiField.extractedTexts, ['Fixed problem']); + + const apiRawField = parseSubcommand("gh api repos/apache/airflow/issues/1/comments --raw-field 'body=Fixed problem'"); + assertDeepStrictEqual(apiRawField.extractedTexts, ['Fixed problem']); }); - test('skips public surface parsing when target is a private tracker repo', () => { - const parsed = parsePublicSurfaceCommand('gh pr comment 123 --repo airflow-s/airflow-s --body "Fixes CVE-2026-1"'); - assertStrictEqual(parsed.isPrivateTrackerTarget, true); - assertStrictEqual(parsed.isPublicSurfaceCommand, false); + test('extracts git -C directory and -R target repo', () => { + const parsed = parseSubcommand('git -C /home/repo commit -m "Fix"'); + assertStrictEqual(parsed.gitCwd, '/home/repo'); + + const ghParsed = parseSubcommand('gh pr create -R apache/airflow -t "T" -b "B"'); + assertStrictEqual(ghParsed.targetRepo, 'apache/airflow'); }); - test('ignores non-public surface commands', () => { - assertStrictEqual(parsePublicSurfaceCommand('pytest tests/').isPublicSurfaceCommand, false); - assertStrictEqual(parsePublicSurfaceCommand('git status').isPublicSurfaceCommand, false); - assertStrictEqual(parsePublicSurfaceCommand('git diff HEAD~1').isPublicSurfaceCommand, false); - assertStrictEqual(parsePublicSurfaceCommand('uv run ruff check').isPublicSurfaceCommand, false); - assertStrictEqual(parsePublicSurfaceCommand('').isPublicSurfaceCommand, false); + test('detects dynamic inputs (heredoc, command substitution, variables, stdin pipe)', () => { + assertStrictEqual(parseSubcommand("git commit -F - << 'EOF'\nEOF").hasDynamicInput, true); + assertStrictEqual(parseSubcommand('git commit -m "$(cat msg.txt)"').hasDynamicInput, true); + assertStrictEqual(parseSubcommand('git commit -m "`echo test`"').hasDynamicInput, true); + assertStrictEqual(parseSubcommand('git commit -m "$COMMIT_MSG"').hasDynamicInput, true); + assertStrictEqual(parseSubcommand('git commit -F -').hasDynamicInput, true); }); }); describe('evaluatePublicSurfaceCommand', () => { + const defaultProject = { + trackerRepo: 'airflow-s/airflow-s', + upstreamRepo: 'apache/airflow', + }; + test('allows clean neutral commit', async () => { - const evaluation = await evaluatePublicSurfaceCommand('git commit -m "Fix asset graph view"'); - assertStrictEqual(evaluation.blocked, false); - assertDeepStrictEqual(evaluation.matchedTerms, []); + const res = await evaluatePublicSurfaceCommand('git commit -m "Fix asset graph view"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, false); + assertDeepStrictEqual(res.matchedTerms, []); }); - test('blocks git commit -am containing CVE reference', async () => { - const evaluation = await evaluatePublicSurfaceCommand('git commit -am "Fix CVE-2026-5555 in connection test"'); - assertStrictEqual(evaluation.blocked, true); - assert(evaluation.matchedTerms.includes('CVE reference')); + test('blocks git commit containing CVE reference', async () => { + const res = await evaluatePublicSurfaceCommand('git commit -m "Fix CVE-2026-1234 in webserver"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + assert(res.reason?.includes('Apache Magpie Security Guard')); + assert(res.reason?.includes('Confidentiality Rule')); }); - test('blocks git -C commit containing hyphenated vulnerability class', async () => { - const evaluation = await evaluatePublicSurfaceCommand('git -C /path/to/upstream commit -m "Fix SQL-injection in query"'); - assertStrictEqual(evaluation.blocked, true); - assert(evaluation.matchedTerms.includes('SQL injection')); - assert(evaluation.reason?.includes('Apache Magpie Security Guard')); - assert(evaluation.reason?.includes('Confidentiality Rule')); + test('blocks git commit with attached -m"..." (no whitespace)', async () => { + const res = await evaluatePublicSurfaceCommand('git commit -m"Fix CVE-2026-5555"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('blocks git commit with attached -am"..." flag', async () => { + const res = await evaluatePublicSurfaceCommand('git commit -am"Prevent critical SSRF bug"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('SSRF')); + }); + + test('blocks git tag with forbidden terms in annotated message', async () => { + const res = await evaluatePublicSurfaceCommand('git tag -a v1.0.0 -m "Release with CVE-2026-1111 fix"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('blocks gh pr edit containing forbidden terms', async () => { + const res = await evaluatePublicSurfaceCommand('gh pr edit 50 -R apache/airflow --title "Fix CVE-2026-9999"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('blocks gh pr review containing forbidden terms', async () => { + const res = await evaluatePublicSurfaceCommand('gh pr review 50 -R apache/airflow --body "Looks good, fixes the SSRF"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('SSRF')); + }); + + test('blocks gh issue comment containing forbidden terms', async () => { + const res = await evaluatePublicSurfaceCommand('gh issue comment 50 -R apache/airflow -b "Confirmed auth-bypass vulnerability"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('auth bypass')); + assert(res.matchedTerms.includes('vulnerability')); }); - test('blocks commit reading message file that contains forbidden terms', async () => { + test('blocks gh api comment creation with forbidden terms', async () => { + const res = await evaluatePublicSurfaceCommand('gh api repos/apache/airflow/issues/1/comments -f body="Fixed CVE-2026-1000"', { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('per-subcommand isolation: blocks leaking subcommand even if previous targets tracker', async () => { + const multiCmd = 'gh pr view 1 -R airflow-s/airflow-s; gh pr comment 2 -R apache/airflow -b "Fixes CVE-2026-9999 SSRF"'; + const res = await evaluatePublicSurfaceCommand(multiCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('SSRF')); + }); + + test('newline separation: detects command separated by newline', async () => { + const newlineCmd = 'git status\ngit commit -m "Fix CVE-2026-1234 SSRF"'; + const res = await evaluatePublicSurfaceCommand(newlineCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('multi-command sequence: git commit clean && gh pr create leaking', async () => { + const seqCmd = 'git commit -m "Neutral message" && gh pr create -R apache/airflow -t "Fix CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(seqCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + }); + + test('fails closed if referenced message file cannot be read', async () => { + const res = await evaluatePublicSurfaceCommand('git commit -F missing.txt', { + projectConfig: defaultProject, + readFile: async () => null, + }); + assertStrictEqual(res.blocked, true); + assert(res.reason?.includes('Referenced message file cannot be read') || res.reason?.includes('could not be read')); + assert(res.reason?.includes('missing.txt')); + }); + + test('resolves file path relative to git -C directory', async () => { const mockFiles: Record = { - '/tmp/msg.txt': 'Stop deserializing XCom values\n\nFixes arbitrary SQL injection in worker.', + '/upstream/msg.txt': 'Neutral headline\n\nFixes critical SSRF vulnerability.', }; - const evaluation = await evaluatePublicSurfaceCommand('git commit -F /tmp/msg.txt', { - readFile: async (p) => mockFiles[p] || '', + const res = await evaluatePublicSurfaceCommand('git -C /upstream commit -F msg.txt', { + projectConfig: defaultProject, + readFile: async (p) => mockFiles[p], + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('SSRF')); + assert(res.matchedTerms.includes('vulnerability')); + }); + + test('blocks dynamic input pipelines when forbidden terms appear in command line', async () => { + const pipeCmd = 'echo "Fixes critical SSRF vulnerability" | gh pr create -R apache/airflow -t "Fix" --body-file -'; + const res = await evaluatePublicSurfaceCommand(pipeCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('SSRF')); + }); + + test('fails closed on indeterminate dynamic inputs (variables, command substitution, stdin, --fill)', async () => { + // Variable in commit message + const varCmd = await evaluatePublicSurfaceCommand('git commit -m "$COMMIT_MSG"', { + projectConfig: defaultProject, + }); + assertStrictEqual(varCmd.blocked, true); + assert(varCmd.matchedTerms.includes('dynamic input')); + assert(varCmd.reason?.includes('indeterminate dynamic input')); + + // Command substitution + const subCmd = await evaluatePublicSurfaceCommand('git commit -m "$(cat /tmp/msg.txt)"', { + projectConfig: defaultProject, + }); + assertStrictEqual(subCmd.blocked, true); + assert(subCmd.matchedTerms.includes('dynamic input')); + + // Piped stdin + const stdinCmd = await evaluatePublicSurfaceCommand('cat msg.txt | git commit -F -', { + projectConfig: defaultProject, }); + assertStrictEqual(stdinCmd.blocked, true); + assert(stdinCmd.matchedTerms.includes('dynamic input')); - assertStrictEqual(evaluation.blocked, true); - assert(evaluation.matchedTerms.includes('SQL injection')); + // gh pr create --fill + const fillCmd = await evaluatePublicSurfaceCommand('gh pr create -R apache/airflow --fill', { + projectConfig: defaultProject, + }); + assertStrictEqual(fillCmd.blocked, true); + assert(fillCmd.matchedTerms.includes('dynamic input')); }); - test('blocks heredoc commit containing forbidden terms', async () => { - const heredocCmd = "git commit -F - << 'EOF'\nFix CVE-2026-1111: Critical SSRF vulnerability\nEOF"; - const evaluation = await evaluatePublicSurfaceCommand(heredocCmd); - assertStrictEqual(evaluation.blocked, true); - assert(evaluation.matchedTerms.includes('CVE reference')); - assert(evaluation.matchedTerms.includes('SSRF')); - assert(evaluation.matchedTerms.includes('vulnerability')); + test('handles git commit --amend: blocks indeterminate reuse; allows clean explicit message', async () => { + // Amend without message flag reuses unverified prior commit -> blocked + const bareAmend = await evaluatePublicSurfaceCommand('git commit --amend', { + projectConfig: defaultProject, + }); + assertStrictEqual(bareAmend.blocked, true); + assert(bareAmend.matchedTerms.includes('dynamic input')); + + const noEditAmend = await evaluatePublicSurfaceCommand('git commit --amend --no-edit', { + projectConfig: defaultProject, + }); + assertStrictEqual(noEditAmend.blocked, true); + assert(noEditAmend.matchedTerms.includes('dynamic input')); + + // Amend with explicit clean message -> allowed + const cleanAmend = await evaluatePublicSurfaceCommand('git commit --amend -m "Update asset graph view"', { + projectConfig: defaultProject, + }); + assertStrictEqual(cleanAmend.blocked, false); + + // Amend with explicit leaking message -> blocked on terms + const leakingAmend = await evaluatePublicSurfaceCommand('git commit --amend -m "Fix CVE-2026-9999 SSRF"', { + projectConfig: defaultProject, + }); + assertStrictEqual(leakingAmend.blocked, true); + assert(leakingAmend.matchedTerms.includes('CVE reference')); + assert(leakingAmend.matchedTerms.includes('SSRF')); }); - test('blocks piped command line containing forbidden terms', async () => { - const pipeCmd = 'echo "Fixes critical SSRF vulnerability" | gh pr create --title "Fix" --body-file -'; - const evaluation = await evaluatePublicSurfaceCommand(pipeCmd); - assertStrictEqual(evaluation.blocked, true); - assert(evaluation.matchedTerms.includes('SSRF')); - assert(evaluation.matchedTerms.includes('vulnerability')); + test('allows commands explicitly targeting the private security tracker', async () => { + const trackerCmd = 'gh pr comment 45 -R airflow-s/airflow-s -b "Confirmed SSRF fix for CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(trackerCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, false); }); - test('allows private tracker commands even when mentioning CVEs and vulnerability classes', async () => { - const trackerCmd = 'gh pr comment 45 --repo apache/airflow-s --body "Confirmed SSRF fix for CVE-2026-1234"'; - const evaluation = await evaluatePublicSurfaceCommand(trackerCmd); - assertStrictEqual(evaluation.blocked, false); + test('allows commands targeting other repositories (adopter docs, framework, etc.) and scopes strictly to upstream_repo', async () => { + // Adopter docs repo is not upstreamRepo -> passes unchecked + const docsRepoCmd = 'gh pr comment 45 -R apache/airflow-site -b "Update CVE-2026-1234 advisory docs"'; + const docsRes = await evaluatePublicSurfaceCommand(docsRepoCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(docsRes.blocked, false); + + // Framework repo is not upstreamRepo -> passes unchecked + const frameworkCmd = 'gh pr comment 1 -R apache/magpie -b "fix(generate-cve-json): CVE parser"'; + const frameworkRes = await evaluatePublicSurfaceCommand(frameworkCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(frameworkRes.blocked, false); + + // Upstream repo is strictly blocked + const upstreamCmd = 'gh pr comment 45 -R apache/airflow -b "Fix CVE-2026-1234"'; + const upstreamRes = await evaluatePublicSurfaceCommand(upstreamCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(upstreamRes.blocked, true); + assert(upstreamRes.matchedTerms.includes('CVE reference')); + + // If upstream_repo happens to contain 'private' or end in '-s', it does NOT fail open on naming + const upstreamWithPrivate = 'gh pr comment 45 -R myorg/foo-private -b "Fix SSRF"'; + const resPrivate = await evaluatePublicSurfaceCommand(upstreamWithPrivate, { + projectConfig: { trackerRepo: 'other-s/other-s', upstreamRepo: 'myorg/foo-private' }, + }); + assertStrictEqual(resPrivate.blocked, true); + assert(resPrivate.matchedTerms.includes('SSRF')); + + // Custom tracker repo not ending in '-s' is properly exempt + const customTrackerCmd = 'gh pr comment 1 -R apache/foo-security -b "Investigating CVE-2026-9999"'; + const customTrackerRes = await evaluatePublicSurfaceCommand(customTrackerCmd, { + projectConfig: { trackerRepo: 'apache/foo-security', upstreamRepo: 'apache/foo' }, + }); + assertStrictEqual(customTrackerRes.blocked, false); + + // When upstreamRepo is unconfigured or is the framework repo itself (apache/magpie), ordinary commits pass + const unconfiguredRes = await evaluatePublicSurfaceCommand('git commit -m "fix(generate-cve-json): handle CVE fields"', { + projectConfig: {}, + }); + assertStrictEqual(unconfiguredRes.blocked, false); + + const magpieRepoRes = await evaluatePublicSurfaceCommand('git commit -m "fix(generate-cve-json): validate CVE schema"', { + projectConfig: { upstreamRepo: 'apache/magpie' }, + }); + assertStrictEqual(magpieRepoRes.blocked, false); + + const magpiePrRes = await evaluatePublicSurfaceCommand('gh pr create -R apache/magpie -t "fix(generate-cve-json): fix vulnerability test"', { + projectConfig: { upstreamRepo: 'apache/magpie' }, + }); + assertStrictEqual(magpiePrRes.blocked, false); + + // git -C in unrelated repo passes; git -C in upstream repo blocks + const unrelatedGitCwd = await evaluatePublicSurfaceCommand('git -C /home/user/magpie commit -m "fix: CVE test"', { + projectConfig: defaultProject, + }); + assertStrictEqual(unrelatedGitCwd.blocked, false); + + const upstreamGitCwd = await evaluatePublicSurfaceCommand('git -C /home/user/airflow commit -m "fix: CVE-2026-1234"', { + projectConfig: defaultProject, + }); + assertStrictEqual(upstreamGitCwd.blocked, true); }); - test('honors maintainer escape hatch via env var', async () => { - const leakingCmd = 'git commit -m "Backport CVE-2026-1234 release notes"'; - const evaluation = await evaluatePublicSurfaceCommand(leakingCmd, { - env: { MAGPIE_ALLOW_EMBARGO_TERMS: '1' }, + test('escape hatch: cannot be bypassed from command-line injection', async () => { + const cmdWithInlineEnv = 'MAGPIE_ALLOW_EMBARGO_TERMS=1 git commit -m "CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(cmdWithInlineEnv, { + projectConfig: defaultProject, }); - assertStrictEqual(evaluation.blocked, false); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE reference')); + + const cmdWithFlag = 'git commit -m "CVE-2026-1234 --allow-embargo-terms"'; + const res2 = await evaluatePublicSurfaceCommand(cmdWithFlag, { + projectConfig: defaultProject, + }); + assertStrictEqual(res2.blocked, true); + assert(res2.matchedTerms.includes('CVE reference')); }); - test('honors maintainer escape hatch via inline flag', async () => { - const leakingCmd = 'MAGPIE_ALLOW_EMBARGO_TERMS=1 git commit -m "Backport CVE-2026-1234"'; - const evaluation = await evaluatePublicSurfaceCommand(leakingCmd); - assertStrictEqual(evaluation.blocked, false); + test('escape hatch: honored ONLY when set in environment by human', async () => { + const cmd = 'git commit -m "Backport CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(cmd, { + projectConfig: defaultProject, + env: { MAGPIE_ALLOW_EMBARGO_TERMS: '1' }, + }); + assertStrictEqual(res.blocked, false); }); - test('passes non-target commands through immediately', async () => { - const evaluation = await evaluatePublicSurfaceCommand('git log -n 5 --grep "CVE-"'); - assertStrictEqual(evaluation.blocked, false); + test('blocking reason does NOT instruct agent how to bypass; tells to ask maintainer', async () => { + const res = await evaluatePublicSurfaceCommand('git commit -m "Fix CVE-2026-1234"', { + projectConfig: defaultProject, + }); + assert(res.reason?.includes('consult an Apache Magpie security-team maintainer to review and set the environment bypass')); + assert(!res.reason?.includes('MAGPIE_ALLOW_EMBARGO_TERMS')); + assert(!res.reason?.includes('--allow-embargo-terms')); }); }); @@ -290,7 +624,7 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { assertStrictEqual(result.success, true); }); - test('intercepts and blocks leaking Bash tool call with combined -am flag returning error', async () => { + test('intercepts and blocks leaking Bash tool call returning error', async () => { let registeredHook: any = null; const on = (event: string, handler: any) => { if (event === 'tool.call') { @@ -300,16 +634,22 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { register(on); + const mockProject = ` +| Key | Value | +| tracker_repo | airflow-s/airflow-s | +| upstream_repo | apache/airflow | +`; + let nextCalled = false; const mockEvent = { tool: 'Bash', - input: { command: 'git -C /upstream commit -am "Fix CVE-2026-1111: SSRF vulnerability"' }, + input: { command: 'gh pr create -R apache/airflow -t "Fix CVE-2026-1111: SSRF vulnerability"' }, }; const next = () => { nextCalled = true; }; - const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); + const result = await registeredHook({ fs: { read: async () => mockProject } }, mockEvent, next); assertStrictEqual(nextCalled, false, 'next() must NOT be called on blocked commands'); assert(result.error, 'Error explanation must be returned'); assert(result.error.includes('Apache Magpie Security Guard')); @@ -317,6 +657,31 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { assert(result.error.includes('SSRF')); }); + test('allows Bash tool call when upstream_repo is not configured (e.g. magpie repo)', async () => { + let registeredHook: any = null; + const on = (event: string, handler: any) => { + if (event === 'tool.call') { + registeredHook = handler; + } + }; + + register(on); + + let nextCalled = false; + const mockEvent = { + tool: 'Bash', + input: { command: 'git commit -m "fix(generate-cve-json): handle CVE fields"' }, + }; + const next = (e: any) => { + nextCalled = true; + return { forwarded: e }; + }; + + const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); + assertStrictEqual(nextCalled, true, 'next() must be called when upstream_repo is not configured'); + assertStrictEqual(result.forwarded.input.command, 'git commit -m "fix(generate-cve-json): handle CVE fields"'); + }); + test('passes non-Bash tools through to next() unchanged', async () => { let registeredHook: any = null; const on = (event: string, handler: any) => { diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index 1149023fb..a12e2bc70 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -19,8 +19,11 @@ /** * Single forbidden-term regexes for public surfaces, aligned with - * plugins/magpie-security/skills/issue-fix/implementation-plan.md:35-43. - * Supports spaces and hyphens in compound vulnerability terms (e.g. SQL-injection). + * plugins/magpie-security/skills/issue-fix/implementation-plan.md:31-43. + * + * Public surface commit messages, PR descriptions, and PR comments must not + * contain any of: CVE references, vulnerability, security fix/patch/advisory, + * security@, or specific vulnerability class names. */ export const FORBIDDEN_TERM_RULES: ReadonlyArray<{ name: string; pattern: RegExp }> = [ { name: 'CVE reference', pattern: /\bCVE(?:-\d{4}-\d+)?\b/i }, @@ -55,82 +58,266 @@ export function findForbiddenTerms(text: string): string[] { return matched; } -export interface ExtractedPublicSurface { - isPublicSurfaceCommand: boolean; - isPrivateTrackerTarget: boolean; - commandType: string; - extractedTexts: string[]; - referencedFiles: string[]; - hasComplexShellInput: boolean; +export type FileReader = (filePath: string) => Promise; + +export interface ProjectRepos { + trackerRepo?: string; + upstreamRepo?: string; + isTrackerCheckout?: boolean; +} + +/** + * Cleans and validates a repository slug (owner/repo). + * Filters out placeholders, TODOs, comments, and non-slug text like "not used". + */ +function cleanRepoSlug(raw: string | undefined): string | undefined { + if (!raw || typeof raw !== 'string') { + return undefined; + } + const trimmed = raw.trim().replace(/^[`'"]+|[`'"]+$/g, '').trim().toLowerCase(); + if ( + trimmed.startsWith('todo') || + trimmed.startsWith('<') || + trimmed.startsWith('not used') || + trimmed.startsWith('not_used') || + trimmed === 'none' || + trimmed === 'n/a' || + trimmed === '-' || + !trimmed.includes('/') + ) { + return undefined; + } + const match = trimmed.match(/^([a-z0-9_.-]+\/[a-z0-9_.-]+)/); + return match ? match[1] : undefined; +} + +/** + * Parses project manifest content (Markdown table or YAML syntax) to extract + * tracker_repo and upstream_repo slugs without assuming naming conventions. + */ +export function parseProjectManifest(content: string): ProjectRepos { + const result: ProjectRepos = {}; + if (!content || typeof content !== 'string') { + return result; + } + + // Markdown table row: | `?tracker_repo`? | `?`? | + const trackerMatch = + content.match(/\|\s*`?tracker_repo`?\s*\|\s*`?([^`|\r\n]+)`?/i) ?? + content.match(/^\s*tracker_repo:\s*`?([^`\r\n#]+)`?/im); + const trackerSlug = cleanRepoSlug(trackerMatch?.[1]); + if (trackerSlug) { + result.trackerRepo = trackerSlug; + } + + // Markdown table row: | `?upstream_repo`? | `?`? | + const upstreamMatch = + content.match(/\|\s*`?upstream_repo`?\s*\|\s*`?([^`|\r\n]+)`?/i) ?? + content.match(/^\s*upstream_repo:\s*`?([^`\r\n#]+)`?/im); + const upstreamSlug = cleanRepoSlug(upstreamMatch?.[1]); + if (upstreamSlug) { + result.upstreamRepo = upstreamSlug; + } + + return result; +} + +/** + * Resolves repository identity from /project.md in precedence order: + * 1. .apache-magpie-local/project.md + * 2. .git/apache-magpie/project.md + * 3. .apache-magpie-overrides/project.md + */ +export async function resolveProjectConfig(readFile?: FileReader): Promise { + if (!readFile) { + return {}; + } + + const candidatePaths = [ + '.apache-magpie-local/project.md', + '.git/apache-magpie/project.md', + '.apache-magpie-overrides/project.md', + ]; + + for (const path of candidatePaths) { + try { + const content = await readFile(path); + if (content && typeof content === 'string') { + const repos = parseProjectManifest(content); + if (repos.trackerRepo || repos.upstreamRepo) { + repos.isTrackerCheckout = true; + return repos; + } + } + } catch { + // Continue to next candidate + } + } + + return {}; } /** - * Checks whether an explicit --repo or -R argument points to a private security tracker. - * ASF security trackers use the `-s/-s` convention or contain `-s` / `security` / `private`. + * Splits a composite shell command line into individual subcommands on + * separators (;, &&, ||, |, \n, \r), respecting single and double quotes. */ -export function isPrivateTrackerTarget(commandLine: string): boolean { +export function splitShellCommands(commandLine: string): string[] { + const commands: string[] = []; if (!commandLine || typeof commandLine !== 'string') { - return false; + return commands; } - const repoMatch = commandLine.match(/(?:--repo|-R)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/i); - if (!repoMatch) { - return false; + let current = ''; + let inSingle = false; + let inDouble = false; + let isEscaped = false; + + for (let i = 0; i < commandLine.length; i++) { + const char = commandLine[i]; + const next = commandLine[i + 1]; + + if (isEscaped) { + current += char; + isEscaped = false; + continue; + } + + if (char === '\\' && !inSingle) { + isEscaped = true; + current += char; + continue; + } + + if (char === "'" && !inDouble) { + inSingle = !inSingle; + current += char; + continue; + } + + if (char === '"' && !inSingle) { + inDouble = !inDouble; + current += char; + continue; + } + + if (!inSingle && !inDouble) { + if (char === '\n' || char === '\r' || char === ';') { + if (current.trim()) { + commands.push(current.trim()); + } + current = ''; + continue; + } + if (char === '&' && next === '&') { + if (current.trim()) { + commands.push(current.trim()); + } + current = ''; + i++; // skip second & + continue; + } + if (char === '|' && next === '|') { + if (current.trim()) { + commands.push(current.trim()); + } + current = ''; + i++; // skip second | + continue; + } + if (char === '|') { + if (current.trim()) { + commands.push(current.trim()); + } + current = ''; + continue; + } + } + + current += char; + } + + if (current.trim()) { + commands.push(current.trim()); } - const repo = (repoMatch[1] ?? repoMatch[2] ?? repoMatch[3] ?? '').toLowerCase(); - if (!repo) { - return false; + return commands; +} + +/** + * Resolves a file path relative to an optional git working directory (-C ) + * without requiring node:path. + */ +export function resolvePath(dir: string | undefined, filePath: string): string { + if (!filePath) return ''; + const trimmed = filePath.trim().replace(/^['"]|['"]$/g, ''); + if (!dir || trimmed.startsWith('/') || trimmed.startsWith('\\') || /^[a-zA-Z]:/.test(trimmed)) { + return trimmed; } + const cleanDir = dir.trim().replace(/[\\/]+$/, ''); + const cleanPath = trimmed.replace(/^\.[\\/]+/, '').replace(/^[\\/]+/, ''); + return `${cleanDir}/${cleanPath}`; +} - // Suffix '-s' (e.g. airflow-s, airflow-s/airflow-s), or names containing private/security tracker - return ( - repo.endsWith('-s') || - repo.includes('-s/') || - repo.includes('/private') || - repo.includes('-private') || - repo.includes('security-tracker') - ); +export interface ParsedSubcommand { + raw: string; + isPublicSurfaceCommand: boolean; + commandType: string; + targetRepo?: string; + gitCwd?: string; + extractedTexts: string[]; + referencedFiles: string[]; + hasDynamicInput: boolean; } /** - * Inspects a Bash command line to determine if it is a public-surface git/gh command, - * extracting any inline commit/PR messages and referenced message files. - * Handles combined short flags (e.g. -am, -sm) and complex shell inputs (heredocs, pipes). + * Parses a single subcommand, identifying git/gh invocations and extracting + * messages (-m, -t, -b, -f body=...), file references (-F, --body-file), + * target repositories (-R, --repo), git working directory (-C), and attached short flags. */ -export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicSurface { - const result: ExtractedPublicSurface = { +export function parseSubcommand(subcommand: string): ParsedSubcommand { + const result: ParsedSubcommand = { + raw: subcommand, isPublicSurfaceCommand: false, - isPrivateTrackerTarget: false, commandType: '', extractedTexts: [], referencedFiles: [], - hasComplexShellInput: false, + hasDynamicInput: false, }; - if (!commandLine || typeof commandLine !== 'string') { + const trimmed = subcommand.trim(); + if (!trimmed) { return result; } - const trimmed = commandLine.trim(); + // Extract git working directory (-C or -C=) + const cwdMatch = trimmed.match(/(?:^|\s)git\s+.*?-C(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/i); + if (cwdMatch) { + result.gitCwd = cwdMatch[1] ?? cwdMatch[2] ?? cwdMatch[3]; + } - // Check if this targets a private tracker (e.g. gh pr comment --repo airflow-s/airflow-s) - if (isPrivateTrackerTarget(trimmed)) { - result.isPrivateTrackerTarget = true; - return result; + // Extract explicit target repository (-R or --repo or --repo=) + const repoMatch = trimmed.match(/(?:(?:^|\s)-(?!-)[a-zA-Z]*R|--repo)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/i); + if (repoMatch) { + result.targetRepo = (repoMatch[1] ?? repoMatch[2] ?? repoMatch[3] ?? '').toLowerCase(); } - // Detect heredocs (<<), shell pipelines (|), or process substitution (<() - const hasComplexShellInput = /<<|\||<\(/.test(trimmed); - result.hasComplexShellInput = hasComplexShellInput; + // Detect dynamic inputs: heredocs (<<), process substitution (<()), command substitution ($() or ``), variables ($VAR) + if (/<<|`|\$\(|\$[a-zA-Z_]/.test(trimmed)) { + result.hasDynamicInput = true; + } - // Match git commit commands, including git -C commit, git -c = commit, git commit -am, etc. - if (/(?:^|[;&|]\s*)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) { + // 1. git commit (including git commit --amend, -m, -am, -sm, -sam, -F) + if (/(?:^|\s)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) { result.isPublicSurfaceCommand = true; result.commandType = 'git commit'; - // Extract -m, -am, -sm, -sam, or --message="..." - const mMatches = trimmed.matchAll(/(?:-[a-zA-Z]*m|--message)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + // Flag git commit --amend without explicit new message as indeterminate reuse + if (/(?:^|\s)--amend(?:\s|$)/i.test(trimmed) && !/(?:(?:^|\s)-(?!-)[a-zA-Z]*[mF]|--message|--file)/.test(trimmed)) { + result.hasDynamicInput = true; + } + + // Extract -m, -am, -sm, -sam, or --message="..." (attached or separated) + const mMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); for (const m of mMatches) { const text = m[1] ?? m[2] ?? m[3]; if (text) { @@ -138,25 +325,101 @@ export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicS } } - // Extract -F or --file= - const fMatches = trimmed.matchAll(/(?:-F|--file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + // Extract -F or --file="..." (attached or separated) + const fMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); for (const f of fMatches) { const file = f[1] ?? f[2] ?? f[3]; - if (file && file !== '-') { - result.referencedFiles.push(file); + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } } } return result; } - // Match gh pr create commands - if (/(?:^|[;&|]\s*)gh\s+pr\s+create(?:\s|$)/i.test(trimmed)) { + // 2. git tag (including git tag -a ... -m "...", git tag -m "...") + if (/(?:^|\s)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*tag(?:\s|$)/i.test(trimmed)) { + // Only tag commands that attach an annotated message (-m or -F) are public-surface publishing commands + const hasMsgFlag = /(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message|(?:^|\s)-(?!-)[a-zA-Z]*F|--file)/.test(trimmed); + if (hasMsgFlag) { + result.isPublicSurfaceCommand = true; + result.commandType = 'git tag'; + + const mMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); + for (const m of mMatches) { + const text = m[1] ?? m[2] ?? m[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + const fMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); + for (const f of fMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + } + } + + return result; + } + } + + // 3. gh pr create + if (/(?:^|\s)gh\s+pr\s+create(?:\s|$)/i.test(trimmed)) { result.isPublicSurfaceCommand = true; result.commandType = 'gh pr create'; - // Title: -t "..." or --title="..." - const titleMatches = trimmed.matchAll(/(?:-t|--title)(?:=|\s+)(?:"([^"]*)"|'([^']*)'|([^\s;]+))/g); + // Flag --fill / --fill-first which auto-populates title and body from commits + if (/(?:^|\s)--fill(?:-first)?(?:\s|$)/i.test(trimmed)) { + result.hasDynamicInput = true; + } + + const titleMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*t|--title)(?:=|\s*|)(?:"([^"]*)"|'([^']*)'|([^\s;&|]+))/g); + for (const t of titleMatches) { + const text = t[1] ?? t[2] ?? t[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); + for (const b of bodyMatches) { + const text = b[1] ?? b[2] ?? b[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); + for (const f of fileMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + } + } + + return result; + } + + // 4. gh pr edit + if (/(?:^|\s)gh\s+pr\s+edit(?:\s|$)/i.test(trimmed)) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr edit'; + + const titleMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*t|--title)(?:=|\s*|)(?:"([^"]*)"|'([^']*)'|([^\s;&|]+))/g); for (const t of titleMatches) { const text = t[1] ?? t[2] ?? t[3]; if (text) { @@ -164,8 +427,35 @@ export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicS } } - // Body: -b "..." or --body="..." - const bodyMatches = trimmed.matchAll(/(?:-b|--body)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); + for (const b of bodyMatches) { + const text = b[1] ?? b[2] ?? b[3]; + if (text) { + result.extractedTexts.push(text); + } + } + + const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); + for (const f of fileMatches) { + const file = f[1] ?? f[2] ?? f[3]; + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + } + } + + return result; + } + + // 5. gh pr review + if (/(?:^|\s)gh\s+pr\s+review(?:\s|$)/i.test(trimmed)) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr review'; + + const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); for (const b of bodyMatches) { const text = b[1] ?? b[2] ?? b[3]; if (text) { @@ -173,25 +463,27 @@ export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicS } } - // Body file: -F or --body-file= - const fileMatches = trimmed.matchAll(/(?:-F|--body-file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); for (const f of fileMatches) { const file = f[1] ?? f[2] ?? f[3]; - if (file && file !== '-') { - result.referencedFiles.push(file); + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } } } return result; } - // Match gh pr comment commands - if (/(?:^|[;&|]\s*)gh\s+pr\s+comment(?:\s|$)/i.test(trimmed)) { + // 6. gh pr comment / gh issue comment + if (/(?:^|\s)gh\s+(?:pr|issue)\s+comment(?:\s|$)/i.test(trimmed)) { result.isPublicSurfaceCommand = true; result.commandType = 'gh pr comment'; - // Body: -b "..." or --body="..." - const bodyMatches = trimmed.matchAll(/(?:-b|--body)(?:=|\s+)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;]+))/g); + const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); for (const b of bodyMatches) { const text = b[1] ?? b[2] ?? b[3]; if (text) { @@ -199,107 +491,234 @@ export function parsePublicSurfaceCommand(commandLine: string): ExtractedPublicS } } - // Body file: -F or --body-file= - const fileMatches = trimmed.matchAll(/(?:-F|--body-file)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;]+))/g); + const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); for (const f of fileMatches) { const file = f[1] ?? f[2] ?? f[3]; - if (file && file !== '-') { - result.referencedFiles.push(file); + if (file) { + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } } } return result; } + // 7. gh api (e.g. gh api repos///... -f body="..." or -F body=...) + if (/(?:^|\s)gh\s+api(?:\s|$)/i.test(trimmed)) { + // Check if targeting a repo endpoint: repos/:owner/:repo + const apiRepoMatch = trimmed.match(/(?:^|\s)gh\s+api\s+(?:repos\/)?([a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+)/i); + if (apiRepoMatch && apiRepoMatch[1]) { + result.targetRepo = apiRepoMatch[1].toLowerCase(); + } + + // Extract fields: -f body=..., -f "body=...", -F body=..., --field body=..., --raw-field body=... + const fieldMatches = trimmed.matchAll( + /(?:-f|-F|--field|--raw-field)(?:=|\s+)(?:"(?:body|title)=([^"]*)"|'(?:body|title)=([^']*)'|(?:body|title)=(?:"([^"]*)"|'([^']*)'|([^\s;&|]+)))/gi + ); + for (const f of fieldMatches) { + const val = f[1] ?? f[2] ?? f[3] ?? f[4] ?? f[5]; + if (val) { + result.extractedTexts.push(val); + } + } + + if (result.extractedTexts.length > 0) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh api'; + return result; + } + } + return result; } -export type FileReader = (filePath: string) => Promise; - export interface EvaluationOptions { readFile?: FileReader; env?: Record; + projectConfig?: ProjectRepos; +} + +/** + * Generates an actionable, maintainer-aligned blocking explanation. + * Does NOT teach the agent how to bypass; instructs asking the maintainer. + */ +function generateBlockingReason(commandType: string, terms: string[]): string { + const termList = terms.map((t) => `\`${t}\``).join(', '); + return [ + `⛔ Apache Magpie Security Guard: Blocked public-surface command (${commandType}).`, + `Found forbidden security disclosure term(s): ${termList}.`, + '', + 'Confidentiality Rule (AGENTS.md & docs/security/fixing-security-issues.md):', + 'Public commits, tags, PR descriptions, and comments on upstream repositories must describe the change in neutral functional terms without mentioning CVE IDs or vulnerability classes before the coordinated disclosure advisory has shipped.', + 'If this is legitimate post-coordinated-disclosure maintenance, consult an Apache Magpie security-team maintainer to review and set the environment bypass.', + ].join('\n'); } /** * Evaluates whether a public-surface command violates embargo terms. - * Returns null if allowed, or an actionable blocking error message if violated. + * Evaluates per subcommand, respects project.md config for tracker/upstream resolution, + * fails closed on unreadable files, and honours only human-set environment escape hatches. */ export async function evaluatePublicSurfaceCommand( commandLine: string, options?: EvaluationOptions ): Promise<{ blocked: boolean; matchedTerms: string[]; reason?: string }> { - // Check for explicit maintainer escape hatch (e.g. post-disclosure release notes or backport PRs) - const envVal = options?.env?.MAGPIE_ALLOW_EMBARGO_TERMS; - if ( - envVal === '1' || - /(?:^|\s)MAGPIE_ALLOW_EMBARGO_TERMS=1(?:\s|$)/.test(commandLine) || - commandLine.includes('--allow-embargo-terms') - ) { + // Deterministic escape hatch: honour ONLY environment variable set by human before session + if (options?.env?.MAGPIE_ALLOW_EMBARGO_TERMS === '1') { return { blocked: false, matchedTerms: [] }; } - const parsed = parsePublicSurfaceCommand(commandLine); - if (!parsed.isPublicSurfaceCommand || parsed.isPrivateTrackerTarget) { + if (!commandLine || typeof commandLine !== 'string') { return { blocked: false, matchedTerms: [] }; } - const allTexts: string[] = [...parsed.extractedTexts]; + // Resolve project configuration (tracker_repo, upstream_repo) + const projectConfig = options?.projectConfig ?? (await resolveProjectConfig(options?.readFile)); + const trackerRepo = projectConfig.trackerRepo?.toLowerCase(); + const upstreamRepo = projectConfig.upstreamRepo?.toLowerCase(); - // If heredocs, pipes, or process substitution are used, scan the whole raw command line as well - if (parsed.hasComplexShellInput) { - allTexts.push(commandLine); + // If no upstream_repo or tracker_repo is configured, or if running in the apache/magpie framework repo, + // this workspace is not bound to a private embargoed fix workflow. Let all pass. + if (!upstreamRepo || !trackerRepo || trackerRepo === upstreamRepo || upstreamRepo === 'apache/magpie') { + return { blocked: false, matchedTerms: [] }; } - // If external message files are referenced, attempt to read their contents - if (options?.readFile && parsed.referencedFiles.length > 0) { + // Split command line into individual subcommands + const subcommands = splitShellCommands(commandLine); + + for (const rawSub of subcommands) { + const parsed = parseSubcommand(rawSub); + if (!parsed.isPublicSurfaceCommand) { + continue; + } + + // 1. Exemption: Subcommand explicitly targets the private security tracker + if (parsed.targetRepo && trackerRepo && parsed.targetRepo === trackerRepo) { + continue; + } + + // 2. Repository scoping: If subcommand targets an explicit repository that is NOT upstream_repo, let it pass + if (parsed.targetRepo && parsed.targetRepo !== upstreamRepo) { + continue; + } + + // 3. For git commands without explicit repo, check repository context + if (!parsed.targetRepo) { + const gitCwd = parsed.gitCwd?.toLowerCase(); + if (gitCwd) { + // Exemption if inside tracker repository + const trackerSlug = trackerRepo.split('/')[1]; + if (trackerSlug && gitCwd.includes(trackerSlug)) { + continue; + } + // Check if git -C points to upstream repository + const upstreamSlug = upstreamRepo.split('/')[1] ?? upstreamRepo; + const isUpstream = gitCwd.includes(upstreamRepo) || gitCwd.includes(upstreamSlug) || gitCwd.includes('upstream'); + if (!isUpstream) { + // Working in an unrelated repository (e.g. apache/magpie) + continue; + } + } else if (projectConfig.isTrackerCheckout) { + // Bare git command in the tracker repository itself without upstream context + continue; + } else if (options?.readFile) { + try { + const gitConfig = await options.readFile('.git/config'); + if (gitConfig && typeof gitConfig === 'string') { + const gitConfigLower = gitConfig.toLowerCase(); + if (trackerRepo && gitConfigLower.includes(trackerRepo)) { + continue; + } + const upstreamSlug = upstreamRepo.split('/')[1] ?? upstreamRepo; + const isUpstream = gitConfigLower.includes(upstreamRepo) || gitConfigLower.includes(upstreamSlug); + if (!isUpstream) { + continue; + } + } + } catch { + // Fall through to inspection if .git/config is absent + } + } + } + + // If dynamic input (stdin -, heredoc, command substitution, variable, pipe, gh pr create --fill, git commit --amend without message) is present + if (parsed.hasDynamicInput) { + // First check if the commandLine itself explicitly carries forbidden terms (e.g. echo "Fix CVE-..." | git commit -F -) + const dynamicTerms = findForbiddenTerms(commandLine); + if (dynamicTerms.length > 0) { + const uniqueTerms = Array.from(new Set(dynamicTerms)); + return { + blocked: true, + matchedTerms: uniqueTerms, + reason: generateBlockingReason(parsed.commandType, uniqueTerms), + }; + } + // If the published content cannot be determined statically, fail closed + return { + blocked: true, + matchedTerms: ['dynamic input'], + reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}) with indeterminate dynamic input. Content constructed from variables ($VAR), command substitution ($(..)), stdin (-), or automated reuse (--amend/--fill) cannot be verified deterministically before publication. Provide the text explicitly or via an inspectable file.`, + }; + } + + // Collect all text candidates for this subcommand + const candidateTexts: string[] = [...parsed.extractedTexts]; + + // Read and inspect any referenced message files (-F / --body-file) for (const file of parsed.referencedFiles) { + const resolvedPath = resolvePath(parsed.gitCwd, file); + if (!options?.readFile) { + return { + blocked: true, + matchedTerms: [], + reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}). Referenced message file cannot be read: \`${resolvedPath}\`.`, + }; + } + try { - const content = await options.readFile(file); - if (content) { - allTexts.push(content); + const content = await options.readFile(resolvedPath); + if (content === null || content === undefined || typeof content !== 'string') { + return { + blocked: true, + matchedTerms: [], + reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}). Referenced message file could not be read: \`${resolvedPath}\`.`, + }; } + candidateTexts.push(content); } catch { - // Safe skip on unreadable file; pure regex inspection continues + return { + blocked: true, + matchedTerms: [], + reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}). Referenced message file could not be read: \`${resolvedPath}\`.`, + }; } } - } - const combinedText = allTexts.join('\n'); - const matchedTerms = findForbiddenTerms(combinedText); - - if (matchedTerms.length === 0) { - return { blocked: false, matchedTerms: [] }; + // Check for forbidden terms in candidate texts + const combinedText = candidateTexts.join('\n'); + const matched = findForbiddenTerms(combinedText); + + if (matched.length > 0) { + const uniqueTerms = Array.from(new Set(matched)); + return { + blocked: true, + matchedTerms: uniqueTerms, + reason: generateBlockingReason(parsed.commandType, uniqueTerms), + }; + } } - const uniqueTerms = Array.from(new Set(matchedTerms)); - const termList = uniqueTerms.map((t) => `\`${t}\``).join(', '); - - const reason = [ - `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}).`, - `Found forbidden security disclosure term(s): ${termList}.`, - '', - 'Confidentiality Rule (AGENTS.md & docs/security/fixing-security-issues.md):', - 'Public commits, PR descriptions, and PR comments on upstream repositories must describe the behavioral change in neutral terms (e.g. "Fix asset graph view leaking DAGs outside permissions").', - 'They must NEVER mention CVE IDs, "security fix", "vulnerability", or vulnerability classes (SSRF, XSS, etc.) before the coordinated disclosure advisory has shipped.', - '', - 'If this is a legitimate post-disclosure change, set MAGPIE_ALLOW_EMBARGO_TERMS=1 to proceed.', - ].join('\n'); - - return { - blocked: true, - matchedTerms: uniqueTerms, - reason, - }; + return { blocked: false, matchedTerms: [] }; } /** * Event hook registration entry point for Claude Code. */ export function register(on: any): void { - // Hook tool.call to guard Bash executions on public surfaces on('tool.call', async ($: any, e: any, next: any) => { - // Check if the tool is Bash and has a command parameter const toolName = (e?.tool || '').toLowerCase(); const command = e?.input?.command || e?.params?.command; @@ -309,13 +728,12 @@ export function register(on: any): void { try { return await $.fs.read(filePath); } catch { - return ''; + return null; } }, }); if (evaluation.blocked && evaluation.reason) { - // Return blocking error response to halt execution and educate the user/agent return { error: evaluation.reason, }; From f863fc8d2beab5089654a1dde7773ec1904264e7 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 20 Oct 2026 11:36:44 +0530 Subject: [PATCH 3/5] fix(security): address maintainer review on canonical terms and fail-closed gates (#1676) - Derive forbidden terms directly from canonical magpie_vcs.publication.DEFAULT_TERMS using tools/dev/sync-forbidden-terms.py, with pre-commit hook to prevent drift. - Tokenize shell commands with POSIX shell-words parser to handle quotes and escaped quotes. - Track directory context across chained sub-commands (cd && git ...). - Parse git -C only from global options and disambiguate clustered/attached flags. - Inspect remotes and worktrees to detect contributor fork checkouts as upstream scope. - Fail closed on indeterminate dynamic input with -F guidance. - Pass environment from hook event and add register()-level tests for MAGPIE_ALLOW_EMBARGO_TERMS. - Document mod architecture, tradeoffs, and escape hatch in secure-agent-setup.md. --- .pre-commit-config.yaml | 6 + docs/setup/secure-agent-setup.md | 57 + .../hooks/forbidden-terms.json | 97 ++ .../magpie-security/hooks/forbidden-terms.ts | 49 + .../hooks/public-surface-scrub.test.ts | 190 ++- .../hooks/public-surface-scrub.ts | 1082 +++++++++++++---- .../magpie-security/skills/issue-fix/SKILL.md | 1 + tools/dev/sync-forbidden-terms.py | 133 ++ tools/dev/tests/test_sync_forbidden_terms.py | 92 ++ 9 files changed, 1409 insertions(+), 298 deletions(-) create mode 100644 plugins/magpie-security/hooks/forbidden-terms.json create mode 100644 plugins/magpie-security/hooks/forbidden-terms.ts create mode 100644 tools/dev/sync-forbidden-terms.py create mode 100644 tools/dev/tests/test_sync_forbidden_terms.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7e3c72a26..f3e38eb3c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -338,6 +338,12 @@ repos: # already corrected and staged-ready instead of as an instruction. - repo: local hooks: + - id: check-forbidden-terms-sync + name: check-forbidden-terms-sync (canonical scrub terms vs publication.py) + language: system + entry: python3 tools/dev/sync-forbidden-terms.py --check + files: ^(tools/vcs/src/magpie_vcs/publication\.py|tools/dev/sync-forbidden-terms\.py|plugins/magpie-security/hooks/forbidden-terms\.(json|ts))$ + pass_filenames: false - id: check-family-plugins name: check-family-plugins (marketplace plugins vs frontmatter) language: system diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index 4c20db77a..d62614ebc 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -35,6 +35,10 @@ - [Extensible — any skill can contribute a guard](#extensible--any-skill-can-contribute-a-guard) - [Install (user-scope)](#install-user-scope-1) - [Verify](#verify-1) + - [Public-surface scrub mod (Claude Code plugin / hook)](#public-surface-scrub-mod-claude-code-plugin--hook) + - [Term canonicalization](#term-canonicalization) + - [Scoping and fail-closed gates](#scoping-and-fail-closed-gates) + - [Trade-offs and runtime characteristics](#trade-offs-and-runtime-characteristics) - [Sandbox-error hint hook](#sandbox-error-hint-hook) - [Why install it](#why-install-it) - [Why install it user-scope, not project-scope](#why-install-it-user-scope-not-project-scope-1) @@ -1778,6 +1782,59 @@ Expected: a JSON object with `permissionDecision: "deny"` and a reason mentioning the `mention` guard. A plain command (`{"tool_input":{"command":"ls"}}`) produces no output and `exit=0`. +## Public-surface scrub mod (Claude Code plugin / hook) + +For Claude Code harnesses using plugins (such as `magpie-security`), +a deterministic TypeScript mod hook +([`plugins/magpie-security/hooks/public-surface-scrub.ts`](../../plugins/magpie-security/hooks/public-surface-scrub.ts)) +intercepts `Bash` tool calls (`git commit`, `git tag`, `gh pr create|edit|review|comment`, `gh api`) +before execution to prevent accidental early disclosure of security vulnerabilities on public surfaces. + +### Term canonicalization + +The mod does not maintain a private list of forbidden terms. The forbidden +disclosure terms and regex patterns are derived directly from the canonical +single source of truth: `magpie_vcs.publication.DEFAULT_TERMS` +in `tools/vcs/src/magpie_vcs/publication.py` (via `tools/dev/sync-forbidden-terms.py` +and tracked by the pre-commit sync check). + +### Scoping and fail-closed gates + +- **Private tracker exemption**: Subcommands explicitly targeting the private security + tracker repository (`tracker_repo` in `project.md`, e.g. `-R airflow-s/airflow-s`) + are exempt. +- **Upstream repository protection**: Subcommands targeting the public codebase + (`upstream_repo` in `project.md`) or operating within an upstream working tree + are scrubbed. Commands targeting unrelated third-party or adopter documentation + repositories pass unchecked. +- **`cd ` transition tracking**: Tracks working directory transitions across + composite subcommands (e.g. `cd ../upstream && git commit -m "..."`). +- **Deterministic git remote verification**: Repository scope is determined by + reading `.git/config` remotes rather than directory-name substring heuristics. +- **Fail-closed on indeterminate scope**: If the target repository scope cannot + be verified deterministically, commands containing embargo terms fail closed. +- **Fail-closed on unreadable files**: Invocations referencing message files + (`-F ` / `--file` / `--body-file`) fail closed if the referenced file cannot + be read. +- **Fail-closed on dynamic input**: Commands constructing commit or PR content + dynamically via variables (`$VAR`), command substitution (`$(...)`), stdin pipes + (`-`), or automatic reuse (`git commit --amend` without an explicit message, + `gh pr create --fill`) cannot be verified statically and fail closed. The block + explanation instructs providing text explicitly or via an inspectable message file + (`-F `). + +### Trade-offs and runtime characteristics + +- **Unsandboxed execution**: Claude Code mods and hooks execute directly in the + local Node runtime outside the container sandbox. +- **File inspection**: The hook inspects message files specified by `-F ` + via the Claude Code plugin filesystem capability (`$.fs.read()`). +- **Human-only escape hatch**: When legitimate post-coordinated-disclosure + maintenance requires mentioning CVE IDs or vulnerability terms on public surfaces, + a human maintainer sets `MAGPIE_ALLOW_EMBARGO_TERMS=1` in the environment + before starting the session. The escape hatch cannot be invoked or faked from + inside the agent's command line or prompt. + ## Sandbox-error hint hook Companion to the *Sandbox-bypass visibility hook* above — a diff --git a/plugins/magpie-security/hooks/forbidden-terms.json b/plugins/magpie-security/hooks/forbidden-terms.json new file mode 100644 index 000000000..8e3a78239 --- /dev/null +++ b/plugins/magpie-security/hooks/forbidden-terms.json @@ -0,0 +1,97 @@ +[ + { + "name": "CVE id", + "pattern": "\\bCVE-\\d{4}-\\d{4,}\\b", + "flags": "i" + }, + { + "name": "CVE", + "pattern": "\\bCVEs?\\b", + "flags": "i" + }, + { + "name": "vulnerability", + "pattern": "\\bvulnerab(?:le|ility|ilities)\\b", + "flags": "i" + }, + { + "name": "security fix", + "pattern": "\\bsecurity[\\s_-]+fix(?:es|ed)?\\b", + "flags": "i" + }, + { + "name": "advisory", + "pattern": "\\badvisor(?:y|ies)\\b", + "flags": "i" + }, + { + "name": "security@", + "pattern": "\\bsecurity@", + "flags": "i" + }, + { + "name": "CVE-tool URL", + "pattern": "cveprocess\\.apache\\.org", + "flags": "i" + }, + { + "name": "exploit", + "pattern": "\\bexploit(?:s|able|ed)?\\b", + "flags": "i" + }, + { + "name": "SSRF", + "pattern": "\\bSSRF\\b", + "flags": "i" + }, + { + "name": "XSS", + "pattern": "\\bXSS\\b", + "flags": "i" + }, + { + "name": "CSRF", + "pattern": "\\bCSRF\\b", + "flags": "i" + }, + { + "name": "RCE", + "pattern": "\\bRCE\\b", + "flags": "i" + }, + { + "name": "SQL injection", + "pattern": "\\bSQL[\\s_-]+injection\\b", + "flags": "i" + }, + { + "name": "path traversal", + "pattern": "\\b(?:path|directory)[\\s_-]+traversal\\b", + "flags": "i" + }, + { + "name": "remote code execution", + "pattern": "\\b(?:remote|arbitrary)[\\s_-]+code[\\s_-]+execution\\b", + "flags": "i" + }, + { + "name": "privilege escalation", + "pattern": "\\bprivilege[\\s_-]+escalation\\b", + "flags": "i" + }, + { + "name": "auth bypass", + "pattern": "\\b(?:auth|authentication|authorization)[\\s_-]+bypass\\b", + "flags": "i" + }, + { + "name": "buffer overflow", + "pattern": "\\b(?:buffer|heap|stack)[\\s_-]+overflow\\b", + "flags": "i" + }, + { + "name": "use-after-free", + "pattern": "\\buse[\\s_-]+after[\\s_-]+free\\b", + "flags": "i" + } +] diff --git a/plugins/magpie-security/hooks/forbidden-terms.ts b/plugins/magpie-security/hooks/forbidden-terms.ts new file mode 100644 index 000000000..a2a6cc3e9 --- /dev/null +++ b/plugins/magpie-security/hooks/forbidden-terms.ts @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Generated from tools/vcs/src/magpie_vcs/publication.py:DEFAULT_TERMS. Do not edit. + +export interface CanonicalForbiddenTerm { + name: string; + pattern: string; + flags: string; +} + +export const CANONICAL_FORBIDDEN_TERMS: ReadonlyArray = [ + + { name: "CVE id", pattern: "\\bCVE-\\d{4}-\\d{4,}\\b", flags: "i" }, + { name: "CVE", pattern: "\\bCVEs?\\b", flags: "i" }, + { name: "vulnerability", pattern: "\\bvulnerab(?:le|ility|ilities)\\b", flags: "i" }, + { name: "security fix", pattern: "\\bsecurity[\\s_-]+fix(?:es|ed)?\\b", flags: "i" }, + { name: "advisory", pattern: "\\badvisor(?:y|ies)\\b", flags: "i" }, + { name: "security@", pattern: "\\bsecurity@", flags: "i" }, + { name: "CVE-tool URL", pattern: "cveprocess\\.apache\\.org", flags: "i" }, + { name: "exploit", pattern: "\\bexploit(?:s|able|ed)?\\b", flags: "i" }, + { name: "SSRF", pattern: "\\bSSRF\\b", flags: "i" }, + { name: "XSS", pattern: "\\bXSS\\b", flags: "i" }, + { name: "CSRF", pattern: "\\bCSRF\\b", flags: "i" }, + { name: "RCE", pattern: "\\bRCE\\b", flags: "i" }, + { name: "SQL injection", pattern: "\\bSQL[\\s_-]+injection\\b", flags: "i" }, + { name: "path traversal", pattern: "\\b(?:path|directory)[\\s_-]+traversal\\b", flags: "i" }, + { name: "remote code execution", pattern: "\\b(?:remote|arbitrary)[\\s_-]+code[\\s_-]+execution\\b", flags: "i" }, + { name: "privilege escalation", pattern: "\\bprivilege[\\s_-]+escalation\\b", flags: "i" }, + { name: "auth bypass", pattern: "\\b(?:auth|authentication|authorization)[\\s_-]+bypass\\b", flags: "i" }, + { name: "buffer overflow", pattern: "\\b(?:buffer|heap|stack)[\\s_-]+overflow\\b", flags: "i" }, + { name: "use-after-free", pattern: "\\buse[\\s_-]+after[\\s_-]+free\\b", flags: "i" }, +]; diff --git a/plugins/magpie-security/hooks/public-surface-scrub.test.ts b/plugins/magpie-security/hooks/public-surface-scrub.test.ts index d893a042d..f8b103ac9 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.test.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.test.ts @@ -59,23 +59,24 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { test('detects CVE references', () => { const hits = findForbiddenTerms('Fix CVE-2026-12345 in connection test'); - assert(hits.includes('CVE reference')); + assert(hits.includes('CVE id')); const bareHits = findForbiddenTerms('Address issue reported as cve'); - assert(bareHits.includes('CVE reference')); + assert(bareHits.includes('CVE')); }); test('detects vulnerability and security fix keywords (spaced and hyphenated)', () => { assert(findForbiddenTerms('Fix critical vulnerability in parsing logic').includes('vulnerability')); + assert(findForbiddenTerms('close vulnerability').includes('vulnerability')); assert(findForbiddenTerms('Security fix for webserver endpoints').includes('security fix')); assert(findForbiddenTerms('Apply security-fix for session timeout').includes('security fix')); - assert(findForbiddenTerms('Security patch for query builder').includes('security fix')); - assert(findForbiddenTerms('Apply critical security-patch').includes('security fix')); - assert(findForbiddenTerms('Security advisory update').includes('security fix')); - assert(findForbiddenTerms('Contact security@apache.org for details').includes('security mailing list')); + assert(findForbiddenTerms('security advisory draft').includes('advisory')); + assert(findForbiddenTerms('Security advisory update').includes('advisory')); + assert(findForbiddenTerms('Contact security@apache.org for details').includes('security@')); }); test('detects vulnerability classes with whitespace and hyphens', () => { assert(findForbiddenTerms('Prevent SSRF in HTTP provider').includes('SSRF')); + assert(findForbiddenTerms('fix: SSRF').includes('SSRF')); assert(findForbiddenTerms('Mitigate stored XSS in log view').includes('XSS')); assert(findForbiddenTerms('Add CSRF token validation').includes('CSRF')); assert(findForbiddenTerms('Prevent SQL injection in search filter').includes('SQL injection')); @@ -83,17 +84,21 @@ describe('Deterministic Public-Surface Scrub mod (family:security)', () => { assert(findForbiddenTerms('Fix path traversal in file download').includes('path traversal')); assert(findForbiddenTerms('Prevent path-traversal in file handler').includes('path traversal')); assert(findForbiddenTerms('Prevent remote code execution in worker').includes('remote code execution')); - assert(findForbiddenTerms('Close RCE vector in serializer').includes('remote code execution')); + assert(findForbiddenTerms('Close RCE vector in serializer').includes('RCE')); assert(findForbiddenTerms('Fix remote-code-execution vulnerability').includes('remote code execution')); + assert(findForbiddenTerms('fix: Remote-Code-Execution bug').includes('remote code execution')); assert(findForbiddenTerms('Fix privilege escalation in role mapper').includes('privilege escalation')); assert(findForbiddenTerms('Prevent privilege-escalation bug').includes('privilege escalation')); assert(findForbiddenTerms('Fix auth bypass in REST API').includes('auth bypass')); assert(findForbiddenTerms('Mitigate auth-bypass vulnerability').includes('auth bypass')); + assert(findForbiddenTerms('auth_bypass').includes('auth bypass')); + assert(findForbiddenTerms('Buffer overflow in memory reader').includes('buffer overflow')); + assert(findForbiddenTerms('Use-after-free in parser loop').includes('use-after-free')); }); test('returns multiple matched terms uniquely', () => { const terms = findForbiddenTerms('Fix CVE-2026-9999: critical SSRF vulnerability allowing auth-bypass'); - assert(terms.includes('CVE reference')); + assert(terms.includes('CVE id')); assert(terms.includes('SSRF')); assert(terms.includes('vulnerability')); assert(terms.includes('auth bypass')); @@ -144,7 +149,7 @@ upstream_repo: apache/kafka assertStrictEqual(parsed.upstreamRepo, 'apache/magpie'); }); - test('resolveProjectConfig probes in precedence order', async () => { + test('resolveProjectConfig probes in precedence order and sets isTrackerCheckout false for parent dirs', async () => { const mockFiles: Record = { '.apache-magpie-overrides/project.md': '| tracker_repo | overrides-s/tracker |\n| upstream_repo | apache/overrides |', }; @@ -153,6 +158,14 @@ upstream_repo: apache/kafka assertStrictEqual(resolved.trackerRepo, 'overrides-s/tracker'); assertStrictEqual(resolved.upstreamRepo, 'apache/overrides'); assertStrictEqual(resolved.isTrackerCheckout, true); + + // Probing via parent/sibling directory (../) must NOT mark current checkout as tracker + const siblingFiles: Record = { + '../.apache-magpie-local/project.md': '| tracker_repo | tracker-s/tracker-s |\n| upstream_repo | apache/upstream |', + }; + const siblingResolved = await resolveProjectConfig(async (p) => siblingFiles[p]); + assertStrictEqual(siblingResolved.trackerRepo, 'tracker-s/tracker-s'); + assertStrictEqual(siblingResolved.isTrackerCheckout, false); }); }); @@ -281,12 +294,13 @@ upstream_repo: apache/kafka assertStrictEqual(ghParsed.targetRepo, 'apache/airflow'); }); - test('detects dynamic inputs (heredoc, command substitution, variables, stdin pipe)', () => { + test('detects dynamic inputs (heredoc, command substitution, variables, stdin pipe, process substitution)', () => { assertStrictEqual(parseSubcommand("git commit -F - << 'EOF'\nEOF").hasDynamicInput, true); assertStrictEqual(parseSubcommand('git commit -m "$(cat msg.txt)"').hasDynamicInput, true); assertStrictEqual(parseSubcommand('git commit -m "`echo test`"').hasDynamicInput, true); assertStrictEqual(parseSubcommand('git commit -m "$COMMIT_MSG"').hasDynamicInput, true); assertStrictEqual(parseSubcommand('git commit -F -').hasDynamicInput, true); + assertStrictEqual(parseSubcommand('git commit -F <(echo test)').hasDynamicInput, true); }); }); @@ -309,7 +323,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); assert(res.reason?.includes('Apache Magpie Security Guard')); assert(res.reason?.includes('Confidentiality Rule')); }); @@ -319,7 +333,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('blocks git commit with attached -am"..." flag', async () => { @@ -335,7 +349,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('blocks gh pr edit containing forbidden terms', async () => { @@ -343,7 +357,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('blocks gh pr review containing forbidden terms', async () => { @@ -368,7 +382,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('per-subcommand isolation: blocks leaking subcommand even if previous targets tracker', async () => { @@ -377,7 +391,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); assert(res.matchedTerms.includes('SSRF')); }); @@ -387,7 +401,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('multi-command sequence: git commit clean && gh pr create leaking', async () => { @@ -396,7 +410,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); }); test('fails closed if referenced message file cannot be read', async () => { @@ -460,6 +474,13 @@ upstream_repo: apache/kafka }); assertStrictEqual(fillCmd.blocked, true); assert(fillCmd.matchedTerms.includes('dynamic input')); + + // Process substitution <(...) + const procSubCmd = await evaluatePublicSurfaceCommand('git commit -F <(echo test)', { + projectConfig: defaultProject, + }); + assertStrictEqual(procSubCmd.blocked, true); + assert(procSubCmd.matchedTerms.includes('dynamic input')); }); test('handles git commit --amend: blocks indeterminate reuse; allows clean explicit message', async () => { @@ -487,7 +508,7 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(leakingAmend.blocked, true); - assert(leakingAmend.matchedTerms.includes('CVE reference')); + assert(leakingAmend.matchedTerms.includes('CVE id')); assert(leakingAmend.matchedTerms.includes('SSRF')); }); @@ -505,29 +526,29 @@ upstream_repo: apache/kafka const docsRes = await evaluatePublicSurfaceCommand(docsRepoCmd, { projectConfig: defaultProject, }); - assertStrictEqual(docsRes.blocked, false); + assertStrictEqual(docsRes.blocked, false, 'docsRes should not be blocked'); // Framework repo is not upstreamRepo -> passes unchecked const frameworkCmd = 'gh pr comment 1 -R apache/magpie -b "fix(generate-cve-json): CVE parser"'; const frameworkRes = await evaluatePublicSurfaceCommand(frameworkCmd, { projectConfig: defaultProject, }); - assertStrictEqual(frameworkRes.blocked, false); + assertStrictEqual(frameworkRes.blocked, false, 'frameworkRes should not be blocked'); // Upstream repo is strictly blocked const upstreamCmd = 'gh pr comment 45 -R apache/airflow -b "Fix CVE-2026-1234"'; const upstreamRes = await evaluatePublicSurfaceCommand(upstreamCmd, { projectConfig: defaultProject, }); - assertStrictEqual(upstreamRes.blocked, true); - assert(upstreamRes.matchedTerms.includes('CVE reference')); + assertStrictEqual(upstreamRes.blocked, true, 'upstreamRes should be blocked'); + assert(upstreamRes.matchedTerms.includes('CVE id')); // If upstream_repo happens to contain 'private' or end in '-s', it does NOT fail open on naming const upstreamWithPrivate = 'gh pr comment 45 -R myorg/foo-private -b "Fix SSRF"'; const resPrivate = await evaluatePublicSurfaceCommand(upstreamWithPrivate, { projectConfig: { trackerRepo: 'other-s/other-s', upstreamRepo: 'myorg/foo-private' }, }); - assertStrictEqual(resPrivate.blocked, true); + assertStrictEqual(resPrivate.blocked, true, 'resPrivate should be blocked'); assert(resPrivate.matchedTerms.includes('SSRF')); // Custom tracker repo not ending in '-s' is properly exempt @@ -535,34 +556,120 @@ upstream_repo: apache/kafka const customTrackerRes = await evaluatePublicSurfaceCommand(customTrackerCmd, { projectConfig: { trackerRepo: 'apache/foo-security', upstreamRepo: 'apache/foo' }, }); - assertStrictEqual(customTrackerRes.blocked, false); + assertStrictEqual(customTrackerRes.blocked, false, 'customTrackerRes should not be blocked'); - // When upstreamRepo is unconfigured or is the framework repo itself (apache/magpie), ordinary commits pass - const unconfiguredRes = await evaluatePublicSurfaceCommand('git commit -m "fix(generate-cve-json): handle CVE fields"', { + // Clean commit in unconfigured repository passes + const unconfiguredCleanRes = await evaluatePublicSurfaceCommand('git commit -m "refactor(json): handle schema validation"', { projectConfig: {}, }); - assertStrictEqual(unconfiguredRes.blocked, false); + assertStrictEqual(unconfiguredCleanRes.blocked, false, 'unconfiguredCleanRes should not be blocked'); - const magpieRepoRes = await evaluatePublicSurfaceCommand('git commit -m "fix(generate-cve-json): validate CVE schema"', { - projectConfig: { upstreamRepo: 'apache/magpie' }, + // Leaking commit in unconfigured repository fails closed + const unconfiguredLeakingRes = await evaluatePublicSurfaceCommand('git commit -m "fix(generate-cve-json): validate CVE schema"', { + projectConfig: {}, }); - assertStrictEqual(magpieRepoRes.blocked, false); + assertStrictEqual(unconfiguredLeakingRes.blocked, true, 'unconfiguredLeakingRes should be blocked'); + // Commands targeting non-upstream repos pass unchecked const magpiePrRes = await evaluatePublicSurfaceCommand('gh pr create -R apache/magpie -t "fix(generate-cve-json): fix vulnerability test"', { - projectConfig: { upstreamRepo: 'apache/magpie' }, + projectConfig: { trackerRepo: 'airflow-s/airflow-s', upstreamRepo: 'apache/airflow' }, }); - assertStrictEqual(magpiePrRes.blocked, false); + assertStrictEqual(magpiePrRes.blocked, false, 'magpiePrRes should not be blocked'); - // git -C in unrelated repo passes; git -C in upstream repo blocks + // git -C in unrelated repo (verified via remote inspection) passes; git -C in upstream repo blocks + const mockGitConfigs: Record = { + '/home/user/magpie/.git/config': '[remote "origin"]\nurl = https://github.com/apache/magpie.git\n', + '/home/user/airflow/.git/config': '[remote "origin"]\nurl = https://github.com/apache/airflow.git\n', + }; const unrelatedGitCwd = await evaluatePublicSurfaceCommand('git -C /home/user/magpie commit -m "fix: CVE test"', { projectConfig: defaultProject, + readFile: async (p) => mockGitConfigs[p], }); - assertStrictEqual(unrelatedGitCwd.blocked, false); + assertStrictEqual(unrelatedGitCwd.blocked, false, 'unrelatedGitCwd should not be blocked'); const upstreamGitCwd = await evaluatePublicSurfaceCommand('git -C /home/user/airflow commit -m "fix: CVE-2026-1234"', { projectConfig: defaultProject, + readFile: async (p) => mockGitConfigs[p], + }); + assertStrictEqual(upstreamGitCwd.blocked, true, 'upstreamGitCwd should be blocked'); + + // Fork of upstream repo (e.g. contributor/airflow) MUST be blocked — cannot fail open! + const forkGitConfigs: Record = { + '/home/user/fork/.git/config': '[remote "origin"]\nurl = https://github.com/contributor/airflow.git\n', + }; + const forkGitCwd = await evaluatePublicSurfaceCommand('git -C /home/user/fork commit -m "Fix CVE-2026-1234 in fork"', { + projectConfig: defaultProject, + readFile: async (p) => forkGitConfigs[p], }); - assertStrictEqual(upstreamGitCwd.blocked, true); + assertStrictEqual(forkGitCwd.blocked, true, 'forkGitCwd should be blocked'); + assert(forkGitCwd.matchedTerms.includes('CVE id')); + + // PR creation targeting a fork of upstream MUST be blocked + const forkPrRes = await evaluatePublicSurfaceCommand('gh pr create -R contributor/airflow -t "Fix CVE-2026-1234"', { + projectConfig: defaultProject, + }); + assertStrictEqual(forkPrRes.blocked, true, 'forkPrRes should be blocked'); + assert(forkPrRes.matchedTerms.includes('CVE id')); + + // Git worktree with .git file pointing to gitdir: ... must be inspected and blocked if upstream + const worktreeConfigs: Record = { + '/home/user/worktree/.git': 'gitdir: /home/user/main-repo/.git/worktrees/fix-branch\n', + '/home/user/main-repo/.git/worktrees/fix-branch/config': '[remote "origin"]\nurl = https://github.com/apache/airflow.git\n', + }; + const worktreeRes = await evaluatePublicSurfaceCommand('git -C /home/user/worktree commit -m "Fix CVE-2026-1234"', { + projectConfig: defaultProject, + readFile: async (p) => worktreeConfigs[p], + }); + assertStrictEqual(worktreeRes.blocked, true, 'worktreeRes should be blocked'); + assert(worktreeRes.matchedTerms.includes('CVE id')); + }); + + test('tracks cd across sub-commands and blocks upstream leaks', async () => { + const mockGitConfigs: Record = { + '/workspace/upstream/.git/config': '[remote "origin"]\nurl = https://github.com/apache/airflow.git\n', + }; + const chainedCdCmd = 'cd /workspace/upstream && git commit -m "Fix CVE-2026-9999"'; + const res = await evaluatePublicSurfaceCommand(chainedCdCmd, { + projectConfig: defaultProject, + readFile: async (p) => mockGitConfigs[p], + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE id')); + }); + + test('git -c config option does not collide with -C directory option', async () => { + const configCmd = 'git -c user.name=bot commit -m "Neutral message"'; + const res = await evaluatePublicSurfaceCommand(configCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, false); + }); + + test('handles escaped quotes in commit messages', async () => { + const escapedQuoteCmd = 'git commit -m "Refactor \\"parser\\" for CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(escapedQuoteCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE id')); + }); + + test('--repo inside a body cannot forge tracker exemption', async () => { + const fakeRepoBody = 'gh pr create -R apache/airflow -b "--repo airflow-s/airflow-s" -t "Fix CVE-2026-1234"'; + const res = await evaluatePublicSurfaceCommand(fakeRepoBody, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.matchedTerms.includes('CVE id')); + }); + + test('dynamic input fail-closed reason mentions -F guidance', async () => { + const dynamicCmd = 'git commit -m "$(cat /tmp/msg.txt)"'; + const res = await evaluatePublicSurfaceCommand(dynamicCmd, { + projectConfig: defaultProject, + }); + assertStrictEqual(res.blocked, true); + assert(res.reason?.includes('-F ')); }); test('escape hatch: cannot be bypassed from command-line injection', async () => { @@ -571,14 +678,14 @@ upstream_repo: apache/kafka projectConfig: defaultProject, }); assertStrictEqual(res.blocked, true); - assert(res.matchedTerms.includes('CVE reference')); + assert(res.matchedTerms.includes('CVE id')); const cmdWithFlag = 'git commit -m "CVE-2026-1234 --allow-embargo-terms"'; const res2 = await evaluatePublicSurfaceCommand(cmdWithFlag, { projectConfig: defaultProject, }); assertStrictEqual(res2.blocked, true); - assert(res2.matchedTerms.includes('CVE reference')); + assert(res2.matchedTerms.includes('CVE id')); }); test('escape hatch: honored ONLY when set in environment by human', async () => { @@ -653,11 +760,11 @@ upstream_repo: apache/kafka assertStrictEqual(nextCalled, false, 'next() must NOT be called on blocked commands'); assert(result.error, 'Error explanation must be returned'); assert(result.error.includes('Apache Magpie Security Guard')); - assert(result.error.includes('CVE reference')); + assert(result.error.includes('CVE id')); assert(result.error.includes('SSRF')); }); - test('allows Bash tool call when upstream_repo is not configured (e.g. magpie repo)', async () => { + test('honors MAGPIE_ALLOW_EMBARGO_TERMS=1 in register() hook event env', async () => { let registeredHook: any = null; const on = (event: string, handler: any) => { if (event === 'tool.call') { @@ -671,6 +778,7 @@ upstream_repo: apache/kafka const mockEvent = { tool: 'Bash', input: { command: 'git commit -m "fix(generate-cve-json): handle CVE fields"' }, + env: { MAGPIE_ALLOW_EMBARGO_TERMS: '1' }, }; const next = (e: any) => { nextCalled = true; @@ -678,7 +786,7 @@ upstream_repo: apache/kafka }; const result = await registeredHook({ fs: { read: async () => '' } }, mockEvent, next); - assertStrictEqual(nextCalled, true, 'next() must be called when upstream_repo is not configured'); + assertStrictEqual(nextCalled, true, 'next() must be called when MAGPIE_ALLOW_EMBARGO_TERMS=1 is set'); assertStrictEqual(result.forwarded.input.command, 'git commit -m "fix(generate-cve-json): handle CVE fields"'); }); diff --git a/plugins/magpie-security/hooks/public-surface-scrub.ts b/plugins/magpie-security/hooks/public-surface-scrub.ts index a12e2bc70..791d638bb 100644 --- a/plugins/magpie-security/hooks/public-surface-scrub.ts +++ b/plugins/magpie-security/hooks/public-surface-scrub.ts @@ -17,33 +17,54 @@ // specific language governing permissions and limitations // under the License. +import { CANONICAL_FORBIDDEN_TERMS, type CanonicalForbiddenTerm } from './forbidden-terms.ts'; + +/** + * Single canonical forbidden-term regexes for public surfaces, derived directly + * from DEFAULT_TERMS in tools/vcs/src/magpie_vcs/publication.py. + */ +export const FORBIDDEN_TERM_RULES: ReadonlyArray<{ name: string; pattern: RegExp }> = + CANONICAL_FORBIDDEN_TERMS.map((term: CanonicalForbiddenTerm) => ({ + name: term.name, + pattern: new RegExp(term.pattern, term.flags), + })); + +/** + * Consecutive words a candidate public text may share with a private text + * before it is reported as a verbatim copy. Aligned with magpie_vcs.publication.VERBATIM_WORDS. + */ +export const VERBATIM_WORDS = 8; + /** - * Single forbidden-term regexes for public surfaces, aligned with - * plugins/magpie-security/skills/issue-fix/implementation-plan.md:31-43. - * - * Public surface commit messages, PR descriptions, and PR comments must not - * contain any of: CVE references, vulnerability, security fix/patch/advisory, - * security@, or specific vulnerability class names. + * Tokenizes text into lowercase alphanumeric words. */ -export const FORBIDDEN_TERM_RULES: ReadonlyArray<{ name: string; pattern: RegExp }> = [ - { name: 'CVE reference', pattern: /\bCVE(?:-\d{4}-\d+)?\b/i }, - { name: 'vulnerability', pattern: /\bvulnerabilit(?:y|ies)\b/i }, - { name: 'security fix', pattern: /\bsecurity[\s-]+(?:fix|patch|advisory)\b/i }, - { name: 'security mailing list', pattern: /\bsecurity@/i }, - { name: 'SSRF', pattern: /\bSSRF\b/i }, - { name: 'XSS', pattern: /\bXSS\b/i }, - { name: 'CSRF', pattern: /\bCSRF\b/i }, - { name: 'SQL injection', pattern: /\bSQL[\s-]+injection\b/i }, - { name: 'path traversal', pattern: /\bpath[\s-]+traversal\b/i }, - { name: 'remote code execution', pattern: /\b(?:remote[\s-]+code[\s-]+execution|RCE)\b/i }, - { name: 'privilege escalation', pattern: /\bprivilege[\s-]+escalation\b/i }, - { name: 'auth bypass', pattern: /\bauth(?:orization|entication)?[\s-]+bypass\b/i }, -]; +function extractWords(text: string): string[] { + const matches = text.toLowerCase().match(/[a-z0-9]+(?:'[a-z]+)?/g); + return matches ? Array.from(matches) : []; +} + +/** + * Computes consecutive n-grams (shingles) of words from text. + */ +function computeShingles(text: string, width: number): Set { + const words = extractWords(text); + const shingles = new Set(); + if (words.length < width) { + return shingles; + } + for (let i = 0; i <= words.length - width; i++) { + shingles.add(words.slice(i, i + width).join(' ')); + } + return shingles; +} /** * Finds all unique forbidden embargo terms that appear in the candidate text. */ -export function findForbiddenTerms(text: string): string[] { +export function findForbiddenTerms( + text: string, + extraOptions?: { reporterNames?: string[]; privateTexts?: string[] } +): string[] { if (!text || typeof text !== 'string') { return []; } @@ -55,6 +76,36 @@ export function findForbiddenTerms(text: string): string[] { } } + // Check reporter names if provided + if (extraOptions?.reporterNames) { + for (const reporter of extraOptions.reporterNames) { + if (reporter && reporter.trim()) { + const escaped = reporter.trim().replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const reporterRx = new RegExp(`\\b${escaped}\\b`, 'i'); + if (reporterRx.test(text)) { + matched.push('reporter name'); + } + } + } + } + + // Check verbatim private text shingles if provided + if (extraOptions?.privateTexts && extraOptions.privateTexts.length > 0) { + const candidateShingles = computeShingles(text, VERBATIM_WORDS); + if (candidateShingles.size > 0) { + for (const privateText of extraOptions.privateTexts) { + if (!privateText) continue; + const privateShingles = computeShingles(privateText, VERBATIM_WORDS); + for (const s of candidateShingles) { + if (privateShingles.has(s)) { + matched.push('verbatim private text'); + break; + } + } + } + } + } + return matched; } @@ -64,13 +115,15 @@ export interface ProjectRepos { trackerRepo?: string; upstreamRepo?: string; isTrackerCheckout?: boolean; + reporterNames?: string[]; + privateTexts?: string[]; } /** * Cleans and validates a repository slug (owner/repo). * Filters out placeholders, TODOs, comments, and non-slug text like "not used". */ -function cleanRepoSlug(raw: string | undefined): string | undefined { +export function cleanRepoSlug(raw: string | undefined): string | undefined { if (!raw || typeof raw !== 'string') { return undefined; } @@ -127,16 +180,23 @@ export function parseProjectManifest(content: string): ProjectRepos { * 1. .apache-magpie-local/project.md * 2. .git/apache-magpie/project.md * 3. .apache-magpie-overrides/project.md + * 4. ../.apache-magpie-overrides/project.md (if in sibling directory) */ -export async function resolveProjectConfig(readFile?: FileReader): Promise { +export async function resolveProjectConfig(readFile?: FileReader, cwd?: string): Promise { if (!readFile) { return {}; } + const prefix = cwd ? `${cwd.replace(/[\\/]+$/, '')}/` : ''; const candidatePaths = [ + `${prefix}.apache-magpie-local/project.md`, + `${prefix}.git/apache-magpie/project.md`, + `${prefix}.apache-magpie-overrides/project.md`, '.apache-magpie-local/project.md', '.git/apache-magpie/project.md', '.apache-magpie-overrides/project.md', + '../.apache-magpie-local/project.md', + '../.apache-magpie-overrides/project.md', ]; for (const path of candidatePaths) { @@ -145,7 +205,9 @@ export async function resolveProjectConfig(readFile?: FileReader): Promise) - * without requiring node:path. + * Tokenizes a single subcommand line into an argv array according to POSIX shell + * quoting and escaping rules. + * Handles single quotes ('...'), double quotes ("..." with \", \\, \$, \`), + * and unquoted whitespace separation. + */ +export function tokenizeShellWords(subcommand: string): string[] { + const tokens: string[] = []; + if (!subcommand || typeof subcommand !== 'string') { + return tokens; + } + + let current = ''; + let inSingle = false; + let inDouble = false; + let isEscaped = false; + let hasToken = false; + + for (let i = 0; i < subcommand.length; i++) { + const char = subcommand[i]; + + if (isEscaped) { + current += char; + hasToken = true; + isEscaped = false; + continue; + } + + if (char === '\\' && !inSingle) { + if (inDouble) { + // In double quotes, only \", \\, \$, \`, \newline are escape sequences + const next = subcommand[i + 1]; + if (next === '"' || next === '\\' || next === '$' || next === '`' || next === '\n') { + isEscaped = true; + continue; + } + current += char; + hasToken = true; + } else { + isEscaped = true; + } + continue; + } + + if (char === "'" && !inDouble) { + inSingle = !inSingle; + hasToken = true; + continue; + } + + if (char === '"' && !inSingle) { + inDouble = !inDouble; + hasToken = true; + continue; + } + + if (/\s/.test(char) && !inSingle && !inDouble) { + if (hasToken || current.length > 0) { + tokens.push(current); + current = ''; + hasToken = false; + } + continue; + } + + current += char; + hasToken = true; + } + + if (hasToken || current.length > 0) { + tokens.push(current); + } + + return tokens; +} + +/** + * Resolves a file path relative to an optional directory without requiring node:path. */ export function resolvePath(dir: string | undefined, filePath: string): string { if (!filePath) return ''; @@ -264,15 +401,16 @@ export interface ParsedSubcommand { commandType: string; targetRepo?: string; gitCwd?: string; + cdDir?: string; extractedTexts: string[]; referencedFiles: string[]; hasDynamicInput: boolean; } /** - * Parses a single subcommand, identifying git/gh invocations and extracting - * messages (-m, -t, -b, -f body=...), file references (-F, --body-file), - * target repositories (-R, --repo), git working directory (-C), and attached short flags. + * Parses a single subcommand using the shell-words argv tokenizer. + * Identifies git/gh invocations and extracts messages, referenced files, + * target repositories, and git working directories. */ export function parseSubcommand(subcommand: string): ParsedSubcommand { const result: ParsedSubcommand = { @@ -289,245 +427,575 @@ export function parseSubcommand(subcommand: string): ParsedSubcommand { return result; } - // Extract git working directory (-C or -C=) - const cwdMatch = trimmed.match(/(?:^|\s)git\s+.*?-C(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/i); - if (cwdMatch) { - result.gitCwd = cwdMatch[1] ?? cwdMatch[2] ?? cwdMatch[3]; + // Detect dynamic inputs in the raw subcommand string: + // heredocs (<<), process substitution (<()), command substitution ($() or ``), variables ($VAR) + if (/<<|`|\$\(|\$[a-zA-Z_]|<\(/.test(trimmed)) { + result.hasDynamicInput = true; } - // Extract explicit target repository (-R or --repo or --repo=) - const repoMatch = trimmed.match(/(?:(?:^|\s)-(?!-)[a-zA-Z]*R|--repo)(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/i); - if (repoMatch) { - result.targetRepo = (repoMatch[1] ?? repoMatch[2] ?? repoMatch[3] ?? '').toLowerCase(); + const tokens = tokenizeShellWords(trimmed); + if (tokens.length === 0) { + return result; } - // Detect dynamic inputs: heredocs (<<), process substitution (<()), command substitution ($() or ``), variables ($VAR) - if (/<<|`|\$\(|\$[a-zA-Z_]/.test(trimmed)) { - result.hasDynamicInput = true; + // Skip leading environment variable assignments (e.g. VAR=val cmd ...) + let cmdIdx = 0; + while (cmdIdx < tokens.length && /^[a-zA-Z_][a-zA-Z0-9_]*=/.test(tokens[cmdIdx])) { + cmdIdx++; + } + if (cmdIdx >= tokens.length) { + return result; } - // 1. git commit (including git commit --amend, -m, -am, -sm, -sam, -F) - if (/(?:^|\s)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*commit(?:\s|$)/i.test(trimmed)) { - result.isPublicSurfaceCommand = true; - result.commandType = 'git commit'; + const binary = tokens[cmdIdx]; - // Flag git commit --amend without explicit new message as indeterminate reuse - if (/(?:^|\s)--amend(?:\s|$)/i.test(trimmed) && !/(?:(?:^|\s)-(?!-)[a-zA-Z]*[mF]|--message|--file)/.test(trimmed)) { - result.hasDynamicInput = true; - } + // Handle cd command for tracking directory state + if (binary === 'cd' && cmdIdx + 1 < tokens.length) { + result.cdDir = tokens[cmdIdx + 1]; + return result; + } + + // Find git binary in tokens + if (binary === 'git') { + let subcmdIndex = -1; + let gitCwd: string | undefined = undefined; + + // Parse git global options before the git subcommand + for (let i = cmdIdx + 1; i < tokens.length; i++) { + const token = tokens[i]; - // Extract -m, -am, -sm, -sam, or --message="..." (attached or separated) - const mMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const m of mMatches) { - const text = m[1] ?? m[2] ?? m[3]; - if (text) { - result.extractedTexts.push(text); + // -C or -C= (case-sensitive!) + if (token === '-C' && i + 1 < tokens.length) { + gitCwd = tokens[i + 1]; + i++; // skip directory argument + continue; + } + if (token.startsWith('-C=')) { + gitCwd = token.slice(3); + continue; } - } - // Extract -F or --file="..." (attached or separated) - const fMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { - if (file === '-') { - result.hasDynamicInput = true; - } else { - result.referencedFiles.push(file); - } + // Skip options that take an argument: -c =, --config-env =, etc. + if (token === '-c' || token === '--config-env') { + i++; // skip config value + continue; } + + // Skip other flags + if (token.startsWith('-')) { + continue; + } + + // First non-flag token is git subcommand + subcmdIndex = i; + break; } - return result; - } + if (gitCwd) { + result.gitCwd = gitCwd; + } - // 2. git tag (including git tag -a ... -m "...", git tag -m "...") - if (/(?:^|\s)git\s+(?:-[^\s]+(?:\s+[^\s]+)?\s+)*tag(?:\s|$)/i.test(trimmed)) { - // Only tag commands that attach an annotated message (-m or -F) are public-surface publishing commands - const hasMsgFlag = /(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message|(?:^|\s)-(?!-)[a-zA-Z]*F|--file)/.test(trimmed); - if (hasMsgFlag) { - result.isPublicSurfaceCommand = true; - result.commandType = 'git tag'; + if (subcmdIndex === -1) { + return result; + } + + const subcmd = tokens[subcmdIndex]; - const mMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*m|--message)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const m of mMatches) { - const text = m[1] ?? m[2] ?? m[3]; - if (text) { - result.extractedTexts.push(text); + // 1. git commit + if (subcmd === 'commit') { + result.isPublicSurfaceCommand = true; + result.commandType = 'git commit'; + + for (let i = subcmdIndex + 1; i < tokens.length; i++) { + const token = tokens[i]; + + if (token === '--amend') { + // If amending without explicit message or file, flag as dynamic reuse + const hasExplicitMessage = tokens.slice(subcmdIndex + 1).some((t) => + t === '-m' || t === '-F' || t.startsWith('-m') || t.startsWith('-F') || + t.startsWith('--message') || t.startsWith('--file') + ); + if (!hasExplicitMessage) { + result.hasDynamicInput = true; + } + continue; } - } - const fMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { + // -F , -F, --file , --file= + if (token === '-F' || token === '--file') { + if (i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + } + continue; + } + if (token.startsWith('--file=')) { + const file = token.slice('--file='.length); if (file === '-') { result.hasDynamicInput = true; } else { result.referencedFiles.push(file); } + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; } - } - return result; - } - } + // -m , -m, --message , --message= + if (token === '-m' || token === '--message') { + if (i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + } + continue; + } + if (token.startsWith('--message=')) { + result.extractedTexts.push(token.slice('--message='.length)); + continue; + } + if (token.startsWith('-m') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } - // 3. gh pr create - if (/(?:^|\s)gh\s+pr\s+create(?:\s|$)/i.test(trimmed)) { - result.isPublicSurfaceCommand = true; - result.commandType = 'gh pr create'; + // Clustered flags with boolean commit flags preceding m or F (e.g. -am, -sam, -aF, -saF) + const clusteredMatch = token.match(/^-[asvqenS]+([mF])(.*)$/); + if (clusteredMatch) { + const flag = clusteredMatch[1]; + const rest = clusteredMatch[2]; + if (flag === 'm') { + if (rest.length > 0) { + result.extractedTexts.push(rest); + } else if (i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + } + continue; + } + if (flag === 'F') { + const file = rest.length > 0 ? rest : (i + 1 < tokens.length ? tokens[++i] : ''); + if (file === '-') { + result.hasDynamicInput = true; + } else if (file) { + result.referencedFiles.push(file); + } + continue; + } + } + } - // Flag --fill / --fill-first which auto-populates title and body from commits - if (/(?:^|\s)--fill(?:-first)?(?:\s|$)/i.test(trimmed)) { - result.hasDynamicInput = true; + return result; } - const titleMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*t|--title)(?:=|\s*|)(?:"([^"]*)"|'([^']*)'|([^\s;&|]+))/g); - for (const t of titleMatches) { - const text = t[1] ?? t[2] ?? t[3]; - if (text) { - result.extractedTexts.push(text); - } - } + // 2. git tag + if (subcmd === 'tag') { + const hasMsg = tokens.slice(subcmdIndex + 1).some((t) => + t === '-m' || t === '-F' || t.startsWith('-m') || t.startsWith('-F') || + t.startsWith('--message') || t.startsWith('--file') + ); + if (hasMsg) { + result.isPublicSurfaceCommand = true; + result.commandType = 'git tag'; + + for (let i = subcmdIndex + 1; i < tokens.length; i++) { + const token = tokens[i]; + + // -F , -F, --file , --file= + if (token === '-F' || token === '--file') { + if (i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + } + continue; + } + if (token.startsWith('--file=')) { + const file = token.slice('--file='.length); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } - const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const b of bodyMatches) { - const text = b[1] ?? b[2] ?? b[3]; - if (text) { - result.extractedTexts.push(text); - } - } + // -m , -m, --message , --message= + if (token === '-m' || token === '--message') { + if (i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + } + continue; + } + if (token.startsWith('--message=')) { + result.extractedTexts.push(token.slice('--message='.length)); + continue; + } + if (token.startsWith('-m') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } - const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fileMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { - if (file === '-') { - result.hasDynamicInput = true; - } else { - result.referencedFiles.push(file); + // Clustered flags + const clusteredMatch = token.match(/^-[asvqenS]+([mF])(.*)$/); + if (clusteredMatch) { + const flag = clusteredMatch[1]; + const rest = clusteredMatch[2]; + if (flag === 'm') { + if (rest.length > 0) { + result.extractedTexts.push(rest); + } else if (i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + } + continue; + } + if (flag === 'F') { + const file = rest.length > 0 ? rest : (i + 1 < tokens.length ? tokens[++i] : ''); + if (file === '-') { + result.hasDynamicInput = true; + } else if (file) { + result.referencedFiles.push(file); + } + continue; + } + } } + + return result; } } return result; } - // 4. gh pr edit - if (/(?:^|\s)gh\s+pr\s+edit(?:\s|$)/i.test(trimmed)) { - result.isPublicSurfaceCommand = true; - result.commandType = 'gh pr edit'; - - const titleMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*t|--title)(?:=|\s*|)(?:"([^"]*)"|'([^']*)'|([^\s;&|]+))/g); - for (const t of titleMatches) { - const text = t[1] ?? t[2] ?? t[3]; - if (text) { - result.extractedTexts.push(text); + // Handle gh binary in tokens + if (binary === 'gh') { + // Extract target repo: -R , -R=, --repo , --repo=, -R + // Linear scan skipping arguments to other flags to prevent reading --repo from inside body/title + for (let i = cmdIdx + 1; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-R' || token === '--repo') && i + 1 < tokens.length) { + result.targetRepo = tokens[i + 1].toLowerCase(); + i++; + continue; } - } - - const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const b of bodyMatches) { - const text = b[1] ?? b[2] ?? b[3]; - if (text) { - result.extractedTexts.push(text); + if (token.startsWith('-R=')) { + result.targetRepo = token.slice(3).toLowerCase(); + continue; } - } - - const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fileMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { - if (file === '-') { - result.hasDynamicInput = true; - } else { - result.referencedFiles.push(file); - } + if (token.startsWith('--repo=')) { + result.targetRepo = token.slice(7).toLowerCase(); + continue; + } + if (token.startsWith('-R') && token.length > 2) { + result.targetRepo = token.slice(2).toLowerCase(); + continue; + } + // Skip flags that take an argument so body or title text cannot be mistaken for flags + if ((token === '-b' || token === '--body' || token === '-t' || token === '--title' || + token === '-F' || token === '--body-file' || token === '-f' || token === '--field' || + token === '--raw-field') && i + 1 < tokens.length) { + i++; + continue; } } - return result; - } + const ghSub1 = tokens[cmdIdx + 1]; + const ghSub2 = tokens[cmdIdx + 2]; - // 5. gh pr review - if (/(?:^|\s)gh\s+pr\s+review(?:\s|$)/i.test(trimmed)) { - result.isPublicSurfaceCommand = true; - result.commandType = 'gh pr review'; + // gh pr create + if (ghSub1 === 'pr' && ghSub2 === 'create') { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr create'; - const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const b of bodyMatches) { - const text = b[1] ?? b[2] ?? b[3]; - if (text) { - result.extractedTexts.push(text); + if (tokens.some((t) => t === '--fill' || t === '--fill-first')) { + result.hasDynamicInput = true; } - } - const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fileMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { - if (file === '-') { - result.hasDynamicInput = true; - } else { - result.referencedFiles.push(file); + for (let i = cmdIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-t' || token === '--title') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--title=')) { + result.extractedTexts.push(token.slice('--title='.length)); + continue; + } + if (token.startsWith('-t') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if ((token === '-b' || token === '--body') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--body=')) { + result.extractedTexts.push(token.slice('--body='.length)); + continue; + } + if (token.startsWith('-b') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if ((token === '-F' || token === '--body-file') && i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + continue; + } + if (token.startsWith('--body-file=')) { + const file = token.slice('--body-file='.length); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; } } - } - return result; - } + return result; + } - // 6. gh pr comment / gh issue comment - if (/(?:^|\s)gh\s+(?:pr|issue)\s+comment(?:\s|$)/i.test(trimmed)) { - result.isPublicSurfaceCommand = true; - result.commandType = 'gh pr comment'; + // gh pr edit + if (ghSub1 === 'pr' && ghSub2 === 'edit') { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr edit'; - const bodyMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*b|--body)(?:=|\s*|)(?:\$?"([^"]*)"|\$?'([^']*)'|([^\s;&|]+))/g); - for (const b of bodyMatches) { - const text = b[1] ?? b[2] ?? b[3]; - if (text) { - result.extractedTexts.push(text); + for (let i = cmdIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-t' || token === '--title') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--title=')) { + result.extractedTexts.push(token.slice('--title='.length)); + continue; + } + if (token.startsWith('-t') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if ((token === '-b' || token === '--body') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--body=')) { + result.extractedTexts.push(token.slice('--body='.length)); + continue; + } + if (token.startsWith('-b') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if ((token === '-F' || token === '--body-file') && i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + continue; + } + if (token.startsWith('--body-file=')) { + const file = token.slice('--body-file='.length); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } } + + return result; } - const fileMatches = trimmed.matchAll(/(?:(?:^|\s)-(?!-)[a-zA-Z]*F|--body-file)(?:=|\s*|)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))/g); - for (const f of fileMatches) { - const file = f[1] ?? f[2] ?? f[3]; - if (file) { - if (file === '-') { - result.hasDynamicInput = true; - } else { - result.referencedFiles.push(file); + // gh pr review + if (ghSub1 === 'pr' && ghSub2 === 'review') { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr review'; + + for (let i = cmdIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-b' || token === '--body') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--body=')) { + result.extractedTexts.push(token.slice('--body='.length)); + continue; + } + if (token.startsWith('-b') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if (token === '-F' && i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; } } + + return result; } - return result; - } + // gh pr comment / gh issue comment + if ((ghSub1 === 'pr' || ghSub1 === 'issue') && ghSub2 === 'comment') { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh pr comment'; - // 7. gh api (e.g. gh api repos///... -f body="..." or -F body=...) - if (/(?:^|\s)gh\s+api(?:\s|$)/i.test(trimmed)) { - // Check if targeting a repo endpoint: repos/:owner/:repo - const apiRepoMatch = trimmed.match(/(?:^|\s)gh\s+api\s+(?:repos\/)?([a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+)/i); - if (apiRepoMatch && apiRepoMatch[1]) { - result.targetRepo = apiRepoMatch[1].toLowerCase(); + for (let i = cmdIdx + 3; i < tokens.length; i++) { + const token = tokens[i]; + if ((token === '-b' || token === '--body') && i + 1 < tokens.length) { + result.extractedTexts.push(tokens[i + 1]); + i++; + continue; + } + if (token.startsWith('--body=')) { + result.extractedTexts.push(token.slice('--body='.length)); + continue; + } + if (token.startsWith('-b') && token.length > 2) { + result.extractedTexts.push(token.slice(2)); + continue; + } + if ((token === '-F' || token === '--body-file') && i + 1 < tokens.length) { + const file = tokens[i + 1]; + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + i++; + continue; + } + if (token.startsWith('--body-file=')) { + const file = token.slice('--body-file='.length); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } + if (token.startsWith('-F') && token.length > 2) { + const file = token.slice(2); + if (file === '-') { + result.hasDynamicInput = true; + } else { + result.referencedFiles.push(file); + } + continue; + } + } + + return result; } - // Extract fields: -f body=..., -f "body=...", -F body=..., --field body=..., --raw-field body=... - const fieldMatches = trimmed.matchAll( - /(?:-f|-F|--field|--raw-field)(?:=|\s+)(?:"(?:body|title)=([^"]*)"|'(?:body|title)=([^']*)'|(?:body|title)=(?:"([^"]*)"|'([^']*)'|([^\s;&|]+)))/gi - ); - for (const f of fieldMatches) { - const val = f[1] ?? f[2] ?? f[3] ?? f[4] ?? f[5]; - if (val) { - result.extractedTexts.push(val); + // gh api + if (ghSub1 === 'api') { + for (let i = cmdIdx + 2; i < tokens.length; i++) { + const token = tokens[i]; + if (token.startsWith('repos/')) { + const slug = token.slice('repos/'.length).split('/')[0] + '/' + token.slice('repos/'.length).split('/')[1]; + result.targetRepo = slug.toLowerCase(); + continue; + } + if (/^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+/.test(token) && !token.startsWith('-')) { + result.targetRepo = token.toLowerCase(); + continue; + } + + if ((token === '-f' || token === '-F' || token === '--field' || token === '--raw-field') && i + 1 < tokens.length) { + const fieldArg = tokens[i + 1]; + const match = fieldArg.match(/^(?:body|title)=(.*)$/s); + if (match) { + result.extractedTexts.push(match[1]); + } + i++; + continue; + } + const fieldMatch = token.match(/^(?:--field|--raw-field)=(?:body|title)=(.*)$/s); + if (fieldMatch) { + result.extractedTexts.push(fieldMatch[1]); + } + } + + if (result.extractedTexts.length > 0) { + result.isPublicSurfaceCommand = true; + result.commandType = 'gh api'; } - } - if (result.extractedTexts.length > 0) { - result.isPublicSurfaceCommand = true; - result.commandType = 'gh api'; return result; } } @@ -539,6 +1007,8 @@ export interface EvaluationOptions { readFile?: FileReader; env?: Record; projectConfig?: ProjectRepos; + privateTexts?: string[]; + reporterNames?: string[]; } /** @@ -557,10 +1027,103 @@ function generateBlockingReason(commandType: string, terms: string[]): string { ].join('\n'); } +/** + * Inspects a directory's git remote URLs to determine if it is tracker, upstream, or third-party. + * Compares against declared trackerRepo and upstreamRepo (including contributor forks). + */ +async function inspectDirectoryRemote( + dir: string | undefined, + projectConfig: ProjectRepos, + readFile?: FileReader +): Promise<{ isTracker: boolean; isUpstream: boolean; determined: boolean }> { + if (!readFile) { + return { isTracker: false, isUpstream: false, determined: false }; + } + + const gitConfigPath = resolvePath(dir, '.git/config'); + let configContent: string | null = null; + try { + const raw = await readFile(gitConfigPath); + if (raw && typeof raw === 'string') { + configContent = raw; + } + } catch { + configContent = null; + } + + // If reading .git/config failed, check if .git is a worktree gitdir file + if (!configContent) { + try { + const gitFilePath = resolvePath(dir, '.git'); + const gitFileContent = await readFile(gitFilePath); + if (gitFileContent && typeof gitFileContent === 'string') { + const gitDirMatch = gitFileContent.match(/gitdir:\s*([^\r\n]+)/i); + if (gitDirMatch) { + const resolvedGitDir = resolvePath(dir, gitDirMatch[1].trim()); + const worktreeConfigPath = resolvePath(resolvedGitDir, 'config'); + const raw = await readFile(worktreeConfigPath); + if (raw && typeof raw === 'string') { + configContent = raw; + } + } + } + } catch { + configContent = null; + } + } + + if (!configContent) { + return { isTracker: false, isUpstream: false, determined: false }; + } + + try { + const configLower = configContent.toLowerCase(); + // Extract url lines under [remote ...] + const urlMatches = configLower.matchAll(/url\s*=\s*([^\r\n]+)/g); + const urls: string[] = []; + for (const m of urlMatches) { + urls.push(m[1].trim()); + } + + if (urls.length === 0) { + return { isTracker: false, isUpstream: false, determined: false }; + } + + const trackerSlug = projectConfig.trackerRepo?.toLowerCase(); + const upstreamSlug = projectConfig.upstreamRepo?.toLowerCase(); + const upstreamRepoName = upstreamSlug?.includes('/') ? upstreamSlug.split('/')[1] : upstreamSlug; + const trackerRepoName = trackerSlug?.includes('/') ? trackerSlug.split('/')[1] : trackerSlug; + + const matchesRepo = (url: string, slug?: string, repoName?: string): boolean => { + if (!url) return false; + const cleanUrl = url.toLowerCase().replace(/\.git$/, ''); + if (slug && cleanUrl.includes(slug)) { + return true; + } + if (repoName && (cleanUrl.endsWith(`/${repoName}`) || cleanUrl.endsWith(`:${repoName}`))) { + return true; + } + return false; + }; + + const isTracker = trackerSlug ? urls.some((u) => matchesRepo(u, trackerSlug, trackerRepoName)) : false; + const isUpstream = upstreamSlug ? urls.some((u) => matchesRepo(u, upstreamSlug, upstreamRepoName)) : false; + + return { + isTracker, + isUpstream, + determined: true, + }; + } catch { + return { isTracker: false, isUpstream: false, determined: false }; + } +} + /** * Evaluates whether a public-surface command violates embargo terms. * Evaluates per subcommand, respects project.md config for tracker/upstream resolution, - * fails closed on unreadable files, and honours only human-set environment escape hatches. + * tracks cd directory transitions across subcommands, fails closed on unreadable files + * and indeterminate repositories, and honours only human-set environment escape hatches. */ export async function evaluatePublicSurfaceCommand( commandLine: string, @@ -575,79 +1138,78 @@ export async function evaluatePublicSurfaceCommand( return { blocked: false, matchedTerms: [] }; } - // Resolve project configuration (tracker_repo, upstream_repo) - const projectConfig = options?.projectConfig ?? (await resolveProjectConfig(options?.readFile)); - const trackerRepo = projectConfig.trackerRepo?.toLowerCase(); - const upstreamRepo = projectConfig.upstreamRepo?.toLowerCase(); - - // If no upstream_repo or tracker_repo is configured, or if running in the apache/magpie framework repo, - // this workspace is not bound to a private embargoed fix workflow. Let all pass. - if (!upstreamRepo || !trackerRepo || trackerRepo === upstreamRepo || upstreamRepo === 'apache/magpie') { - return { blocked: false, matchedTerms: [] }; - } - // Split command line into individual subcommands const subcommands = splitShellCommands(commandLine); + let virtualCwd: string | undefined = undefined; + for (const rawSub of subcommands) { const parsed = parseSubcommand(rawSub); + + // Track cd transitions + if (parsed.cdDir) { + virtualCwd = resolvePath(virtualCwd, parsed.cdDir); + continue; + } + if (!parsed.isPublicSurfaceCommand) { continue; } + const effectiveDir = parsed.gitCwd || virtualCwd; + + // Resolve project configuration (tracker_repo, upstream_repo) for this subcommand's scope + const projectConfig = options?.projectConfig ?? (await resolveProjectConfig(options?.readFile, effectiveDir)); + const trackerRepo = projectConfig.trackerRepo?.toLowerCase(); + const upstreamRepo = projectConfig.upstreamRepo?.toLowerCase(); + const upstreamRepoName = upstreamRepo?.includes('/') ? upstreamRepo.split('/')[1] : upstreamRepo; + // 1. Exemption: Subcommand explicitly targets the private security tracker if (parsed.targetRepo && trackerRepo && parsed.targetRepo === trackerRepo) { continue; } - // 2. Repository scoping: If subcommand targets an explicit repository that is NOT upstream_repo, let it pass - if (parsed.targetRepo && parsed.targetRepo !== upstreamRepo) { + // 2. Repository scoping: Subcommands targeting upstream or a contributor's fork of upstream must be scrubbed. + // Subcommands targeting an explicit unrelated repository (e.g. adopter docs, framework) pass unchecked. + const isTargetUpstreamOrFork = + parsed.targetRepo && + upstreamRepo && + (parsed.targetRepo === upstreamRepo || + (upstreamRepoName && parsed.targetRepo.endsWith(`/${upstreamRepoName}`))); + + if (parsed.targetRepo && !isTargetUpstreamOrFork) { continue; } - // 3. For git commands without explicit repo, check repository context + // 3. For commands without explicit repo flag, determine repository from git context if (!parsed.targetRepo) { - const gitCwd = parsed.gitCwd?.toLowerCase(); - if (gitCwd) { - // Exemption if inside tracker repository - const trackerSlug = trackerRepo.split('/')[1]; - if (trackerSlug && gitCwd.includes(trackerSlug)) { - continue; - } - // Check if git -C points to upstream repository - const upstreamSlug = upstreamRepo.split('/')[1] ?? upstreamRepo; - const isUpstream = gitCwd.includes(upstreamRepo) || gitCwd.includes(upstreamSlug) || gitCwd.includes('upstream'); - if (!isUpstream) { - // Working in an unrelated repository (e.g. apache/magpie) - continue; - } - } else if (projectConfig.isTrackerCheckout) { - // Bare git command in the tracker repository itself without upstream context + // If we are in the tracker repository checkout itself without upstream context, exempt it + if (projectConfig.isTrackerCheckout && !effectiveDir) { continue; - } else if (options?.readFile) { - try { - const gitConfig = await options.readFile('.git/config'); - if (gitConfig && typeof gitConfig === 'string') { - const gitConfigLower = gitConfig.toLowerCase(); - if (trackerRepo && gitConfigLower.includes(trackerRepo)) { - continue; - } - const upstreamSlug = upstreamRepo.split('/')[1] ?? upstreamRepo; - const isUpstream = gitConfigLower.includes(upstreamRepo) || gitConfigLower.includes(upstreamSlug); - if (!isUpstream) { - continue; - } + } + + // If an effective directory is targeted or we have git config, inspect its remote + if (options?.readFile) { + const remoteInfo = await inspectDirectoryRemote(effectiveDir, projectConfig, options.readFile); + if (remoteInfo.determined) { + if (remoteInfo.isTracker) { + continue; // Safe tracker repository + } + // If verified not upstream (and not a fork of upstream) and not tracker, allow + if (!remoteInfo.isUpstream && upstreamRepo) { + continue; } - } catch { - // Fall through to inspection if .git/config is absent } } } // If dynamic input (stdin -, heredoc, command substitution, variable, pipe, gh pr create --fill, git commit --amend without message) is present if (parsed.hasDynamicInput) { - // First check if the commandLine itself explicitly carries forbidden terms (e.g. echo "Fix CVE-..." | git commit -F -) - const dynamicTerms = findForbiddenTerms(commandLine); + // First check if the commandLine itself explicitly carries forbidden terms + const dynamicTerms = findForbiddenTerms(commandLine, { + reporterNames: options?.reporterNames ?? projectConfig.reporterNames, + privateTexts: options?.privateTexts ?? projectConfig.privateTexts, + }); if (dynamicTerms.length > 0) { const uniqueTerms = Array.from(new Set(dynamicTerms)); return { @@ -660,7 +1222,7 @@ export async function evaluatePublicSurfaceCommand( return { blocked: true, matchedTerms: ['dynamic input'], - reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}) with indeterminate dynamic input. Content constructed from variables ($VAR), command substitution ($(..)), stdin (-), or automated reuse (--amend/--fill) cannot be verified deterministically before publication. Provide the text explicitly or via an inspectable file.`, + reason: `⛔ Apache Magpie Security Guard: Blocked public-surface command (${parsed.commandType}) with indeterminate dynamic input. Content constructed from variables ($VAR), command substitution ($(..)), stdin (-), or automated reuse (--amend/--fill) cannot be verified deterministically before publication. Provide the text explicitly or via an inspectable file (-F ).`, }; } @@ -669,7 +1231,7 @@ export async function evaluatePublicSurfaceCommand( // Read and inspect any referenced message files (-F / --body-file) for (const file of parsed.referencedFiles) { - const resolvedPath = resolvePath(parsed.gitCwd, file); + const resolvedPath = resolvePath(effectiveDir, file); if (!options?.readFile) { return { blocked: true, @@ -697,11 +1259,15 @@ export async function evaluatePublicSurfaceCommand( } } - // Check for forbidden terms in candidate texts + // Check for forbidden terms, reporter names, and verbatim private text const combinedText = candidateTexts.join('\n'); - const matched = findForbiddenTerms(combinedText); + const matched = findForbiddenTerms(combinedText, { + reporterNames: options?.reporterNames ?? projectConfig.reporterNames, + privateTexts: options?.privateTexts ?? projectConfig.privateTexts, + }); if (matched.length > 0) { + // If scope could not be determined deterministically as tracker, fail closed const uniqueTerms = Array.from(new Set(matched)); return { blocked: true, @@ -723,7 +1289,9 @@ export function register(on: any): void { const command = e?.input?.command || e?.params?.command; if (toolName === 'bash' && typeof command === 'string') { + const hookEnv = e?.env ?? (typeof process !== 'undefined' ? process.env : {}); const evaluation = await evaluatePublicSurfaceCommand(command, { + env: hookEnv, readFile: async (filePath: string) => { try { return await $.fs.read(filePath); diff --git a/plugins/magpie-security/skills/issue-fix/SKILL.md b/plugins/magpie-security/skills/issue-fix/SKILL.md index 86f8dd92f..751ed15d7 100644 --- a/plugins/magpie-security/skills/issue-fix/SKILL.md +++ b/plugins/magpie-security/skills/issue-fix/SKILL.md @@ -444,6 +444,7 @@ If it is not `clean`, abort and tell the user. When the framework's secure setup is installed, the agent-guard `security-language` guard ([`guards/security_language.py`](guards/security_language.py)) also blocks a `gh pr create` / `gh pr edit` whose title or body carries a CVE ID, `security fix` or a vulnerability-class name. It does not match the bare words `vulnerability` or `advisory`, which would block ordinary PRs everywhere the guard runs, so those rely on this manual check. It is a backstop, not a replacement for this check: it covers only a subset of the 5c list, and it does not see the commit message, the branch name, or a newsfragment. +In Claude Code sessions with the `magpie-security` plugin enabled, the deterministic public-surface scrub mod ([`hooks/public-surface-scrub.ts`](../../hooks/public-surface-scrub.ts)) automatically intercepts and gates `git commit`, `git tag`, `gh pr create|edit|review|comment`, and `gh api` calls to prevent early disclosure across the full canonical forbidden term list. After the user submits the PR, capture its URL (from the browser, or `gh pr view --json url --jq .url`) for Step 10. diff --git a/tools/dev/sync-forbidden-terms.py b/tools/dev/sync-forbidden-terms.py new file mode 100644 index 000000000..1531b812a --- /dev/null +++ b/tools/dev/sync-forbidden-terms.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +# https://www.apache.org/licenses/LICENSE-2.0 +"""Sync and check the canonical forbidden-term list for public surface scrubbing. + +Source of truth: `DEFAULT_TERMS` in `tools/vcs/src/magpie_vcs/publication.py`. +Output targets: +- `plugins/magpie-security/hooks/forbidden-terms.json` +- `plugins/magpie-security/hooks/forbidden-terms.ts` + +Usage: + python tools/dev/sync-forbidden-terms.py # Write updated files + python tools/dev/sync-forbidden-terms.py --check # Fail if drifted +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +VCS_SRC = REPO_ROOT / "tools" / "vcs" / "src" +TARGET_JSON = REPO_ROOT / "plugins" / "magpie-security" / "hooks" / "forbidden-terms.json" +TARGET_TS = REPO_ROOT / "plugins" / "magpie-security" / "hooks" / "forbidden-terms.ts" + +if str(VCS_SRC) not in sys.path: + sys.path.insert(0, str(VCS_SRC)) + +try: + from magpie_vcs.publication import DEFAULT_TERMS +except ImportError as err: + sys.stderr.write(f"sync-forbidden-terms: failed to import magpie_vcs: {err}\n") + sys.exit(1) + +TS_HEADER = """// SPDX-License-Identifier: Apache-2.0 +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Generated from tools/vcs/src/magpie_vcs/publication.py:DEFAULT_TERMS. Do not edit. + +export interface CanonicalForbiddenTerm { + name: string; + pattern: string; + flags: string; +} + +export const CANONICAL_FORBIDDEN_TERMS: ReadonlyArray = [ +""" + + +def generate_terms_data() -> list[dict[str, str]]: + """Convert DEFAULT_TERMS into canonical serializable objects.""" + terms: list[dict[str, str]] = [] + for label, pattern in DEFAULT_TERMS: + terms.append( + { + "name": label, + "pattern": pattern, + "flags": "i", + } + ) + return terms + + +def generate_ts_content(terms: list[dict[str, str]]) -> str: + lines = [TS_HEADER] + for term in terms: + name_esc = json.dumps(term["name"]) + pat_esc = json.dumps(term["pattern"]) + flags_esc = json.dumps(term["flags"]) + lines.append(f" {{ name: {name_esc}, pattern: {pat_esc}, flags: {flags_esc} }},") + lines.append("];\n") + return "\n".join(lines) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--check", + action="store_true", + help="Check if forbidden-terms files match DEFAULT_TERMS without writing.", + ) + args = parser.parse_args() + + data = generate_terms_data() + json_serialized = json.dumps(data, indent=2) + "\n" + ts_serialized = generate_ts_content(data) + + if args.check: + if not TARGET_JSON.exists() or not TARGET_TS.exists(): + sys.stderr.write( + "sync-forbidden-terms: target files do not exist. " + "Run 'python tools/dev/sync-forbidden-terms.py' to generate.\n" + ) + return 1 + + existing_json = TARGET_JSON.read_text(encoding="utf-8") + existing_ts = TARGET_TS.read_text(encoding="utf-8") + if existing_json != json_serialized or existing_ts != ts_serialized: + sys.stderr.write( + "sync-forbidden-terms: targets have drifted from DEFAULT_TERMS. " + "Run 'python tools/dev/sync-forbidden-terms.py' to synchronize.\n" + ) + return 1 + + return 0 + + TARGET_JSON.parent.mkdir(parents=True, exist_ok=True) + TARGET_JSON.write_text(json_serialized, encoding="utf-8") + TARGET_TS.write_text(ts_serialized, encoding="utf-8") + print(f"Synchronized {len(data)} canonical forbidden terms to {TARGET_JSON.name} and {TARGET_TS.name}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/dev/tests/test_sync_forbidden_terms.py b/tools/dev/tests/test_sync_forbidden_terms.py new file mode 100644 index 000000000..ff4ab092c --- /dev/null +++ b/tools/dev/tests/test_sync_forbidden_terms.py @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for `tools/dev/sync-forbidden-terms.py`.""" + +from __future__ import annotations + +import importlib.util +import json +import subprocess +import sys +from pathlib import Path +from types import ModuleType + +_SCRIPT = Path(__file__).resolve().parents[1] / "sync-forbidden-terms.py" +REPO_ROOT = Path(__file__).resolve().parents[3] +TARGET_JSON = REPO_ROOT / "plugins" / "magpie-security" / "hooks" / "forbidden-terms.json" +TARGET_TS = REPO_ROOT / "plugins" / "magpie-security" / "hooks" / "forbidden-terms.ts" + + +def _load() -> ModuleType: + spec = importlib.util.spec_from_file_location("sync_forbidden_terms", _SCRIPT) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_terms_match_canonical_publication_terms() -> None: + mod = _load() + terms = mod.generate_terms_data() + assert len(terms) >= 19 + names = {t["name"] for t in terms} + assert "CVE id" in names + assert "CVE" in names + assert "vulnerability" in names + assert "security fix" in names + assert "advisory" in names + assert "exploit" in names + assert "SSRF" in names + assert "buffer overflow" in names + assert "use-after-free" in names + + +def test_sync_script_check_passes_on_current_tree() -> None: + result = subprocess.run( + [sys.executable, str(_SCRIPT), "--check"], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + assert result.returncode == 0, f"sync-forbidden-terms --check failed: {result.stderr}" + + +def test_sync_script_check_fails_on_json_drift(tmp_path: Path, monkeypatch) -> None: + mod = _load() + fake_json = tmp_path / "forbidden-terms.json" + fake_json.write_text("[]\n", encoding="utf-8") + fake_ts = tmp_path / "forbidden-terms.ts" + fake_ts.write_text(mod.generate_ts_content(mod.generate_terms_data()), encoding="utf-8") + + monkeypatch.setattr(mod, "TARGET_JSON", fake_json) + monkeypatch.setattr(mod, "TARGET_TS", fake_ts) + monkeypatch.setattr(sys, "argv", [str(_SCRIPT), "--check"]) + assert mod.main() == 1 + + +def test_sync_script_check_fails_on_ts_drift(tmp_path: Path, monkeypatch) -> None: + mod = _load() + fake_json = tmp_path / "forbidden-terms.json" + fake_json.write_text(json.dumps(mod.generate_terms_data(), indent=2) + "\n", encoding="utf-8") + fake_ts = tmp_path / "forbidden-terms.ts" + fake_ts.write_text("// drifted\n", encoding="utf-8") + + monkeypatch.setattr(mod, "TARGET_JSON", fake_json) + monkeypatch.setattr(mod, "TARGET_TS", fake_ts) + monkeypatch.setattr(sys, "argv", [str(_SCRIPT), "--check"]) + assert mod.main() == 1 From 4a89ad2875fc9c2607bdebfe9127489207bcaf06 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 20 Oct 2026 11:56:01 +0530 Subject: [PATCH 4/5] docs: document sync-forbidden-terms.py and update issue-fix token count stamp --- plugins/magpie-security/skills/issue-fix/SKILL.md | 2 +- tools/dev/README.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/magpie-security/skills/issue-fix/SKILL.md b/plugins/magpie-security/skills/issue-fix/SKILL.md index 751ed15d7..26bbd0583 100644 --- a/plugins/magpie-security/skills/issue-fix/SKILL.md +++ b/plugins/magpie-security/skills/issue-fix/SKILL.md @@ -23,7 +23,7 @@ capability: - capability:resolve surface_hash: sha256:550eabe3b9bb3689 license: Apache-2.0 -measured_tokens: 7331 +measured_tokens: 7415 ---