From 119516037777d56bf8a5fcef3fb739638ac7987f Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 27 May 2026 20:14:45 -0700 Subject: [PATCH] ci(adapters): reject git push in adapter/runtime code (PAPA-442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PAPA-432 established the no-remote-git contract — the local execution- workspace cwd is the only persistence boundary across runs, so adapter and runtime code must not push to a git remote. This adds a static check that fails CI when `git push` (string-form invocations or args-array form for spawn/execFile) is added to `packages/adapters/`, `packages/adapter-utils/`, `server/src/`, or `cli/src/`. - `scripts/check-no-git-push.mjs` scans the scoped directories and flags any unapproved match. `//` comments are ignored so doc mentions of "git push" (e.g. ssh-fixture.test.ts) don't trip the rule. - Operators can opt in per-call via `// paperclip:allow-git-push: ` on the line or the line above, forcing the exception to surface in code review. - Wired into the `policy` PR job (so it runs in <1s, before heavier jobs) and into `test:release-registry` so release tooling re-validates the rule. - Release scripts (`scripts/release.sh`) are out of scope by design — they push to publish artifacts, which is operator-driven and lives outside the adapter/runtime tree. Co-Authored-By: Paperclip --- .github/workflows/pr.yml | 6 + package.json | 4 +- scripts/check-no-git-push.mjs | 189 +++++++++++++++++++++++++++++ scripts/check-no-git-push.test.mjs | 162 +++++++++++++++++++++++++ 4 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 scripts/check-no-git-push.mjs create mode 100644 scripts/check-no-git-push.test.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index fa0817968d9..9f5ec579cae 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -45,6 +45,12 @@ jobs: - name: Validate Dockerfile deps stage run: node ./scripts/check-docker-deps-stage.mjs + - name: Reject git push in adapter/runtime code + run: node ./scripts/check-no-git-push.mjs + + - name: Test no-git-push check + run: node --test ./scripts/check-no-git-push.test.mjs + - name: Validate release package manifest run: node ./scripts/release-package-map.mjs check diff --git a/package.json b/package.json index 60b80d5a319..4c613c5cf44 100644 --- a/package.json +++ b/package.json @@ -35,12 +35,14 @@ "release:rollback": "./scripts/rollback-latest.sh", "release:bootstrap-package": "node scripts/bootstrap-npm-package.mjs", "check:tokens": "node scripts/check-forbidden-tokens.mjs", + "check:no-git-push": "node scripts/check-no-git-push.mjs", + "test:check-no-git-push": "node --test scripts/check-no-git-push.test.mjs", "docs:dev": "cd docs && npx mintlify dev", "smoke:openclaw-join": "./scripts/smoke/openclaw-join.sh", "smoke:openclaw-docker-ui": "./scripts/smoke/openclaw-docker-ui.sh", "smoke:openclaw-sse-standalone": "./scripts/smoke/openclaw-sse-standalone.sh", "smoke:terminal-bench-loop-skill": "node scripts/smoke/terminal-bench-loop-skill-smoke.mjs", - "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs", + "test:release-registry": "node --test scripts/verify-release-registry-state.test.mjs scripts/release-package-map.test.mjs scripts/check-release-package-bootstrap.test.mjs scripts/check-no-git-push.test.mjs", "test:e2e": "npx playwright test --config tests/e2e/playwright.config.ts", "test:e2e:headed": "npx playwright test --config tests/e2e/playwright.config.ts --headed", "test:e2e:multiuser-authenticated": "npx playwright test --config tests/e2e/playwright-multiuser-authenticated.config.ts", diff --git a/scripts/check-no-git-push.mjs b/scripts/check-no-git-push.mjs new file mode 100644 index 00000000000..5cdc06b8ae7 --- /dev/null +++ b/scripts/check-no-git-push.mjs @@ -0,0 +1,189 @@ +#!/usr/bin/env node +/** + * check-no-git-push.mjs + * + * Static check that rejects `git push` (and equivalent remote-mutating git + * invocations) inside adapter/runtime source code. + * + * Adapter and runtime code may never push to a git remote: the local + * execution-workspace cwd is the only persistence boundary between runs + * (see packages/adapters/AUTHORING.md and PAPA-432). Release tooling and + * developer scripts that legitimately push are out of scope because they + * live outside the directories scanned here. + * + * Opt-in mechanism: a line containing `paperclip:allow-git-push` (typically + * inside a `// paperclip:allow-git-push: ` comment on the line itself + * or the line immediately above) suppresses the match. This is reserved for + * operator-configured paths that legitimately push and must be reviewed. + */ + +import { readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; +import process from "node:process"; +import { fileURLToPath } from "node:url"; + +const DEFAULT_SCAN_ROOTS = [ + "packages/adapters", + "packages/adapter-utils", + "server/src", + "cli/src", +]; + +const SCANNABLE_EXTENSIONS = new Set([".ts", ".tsx", ".js", ".mjs", ".cjs"]); + +const SKIP_DIRECTORY_NAMES = new Set([ + "node_modules", + "dist", + "build", + ".turbo", + ".next", + "coverage", +]); + +const SKIP_FILENAME_SUFFIXES = [".d.ts"]; + +// Matches actual git push invocations in either: +// `git push ...` (shell command string) +// ["git", "push", ...] (args-array form for execSync) +// execFile("git", ["push", ...]) / spawn("git", ["push", ...]) +export const GIT_PUSH_PATTERNS = [ + /\bgit[\s_-]+push\b/i, + /["'`]git["'`]\s*,\s*\[?\s*["'`]push["'`]/i, +]; +// Kept for backwards-compatibility with existing tests/importers. +export const GIT_PUSH_PATTERN = GIT_PUSH_PATTERNS[0]; +export const ALLOW_MARKER = "paperclip:allow-git-push"; + +function lineMatchesGitPush(line) { + return GIT_PUSH_PATTERNS.some((pattern) => pattern.test(line)); +} + +function stripLineComment(line) { + // Strip everything from the first `//` that is not inside a string literal. + // This is a lightweight heuristic: we only need to remove obvious doc-style + // mentions of "git push" so they do not trip the check. The check still + // flags any match that survives comment stripping. + let inSingle = false; + let inDouble = false; + let inBacktick = false; + + for (let index = 0; index < line.length; index += 1) { + const char = line[index]; + const prev = index > 0 ? line[index - 1] : ""; + + if (!inDouble && !inBacktick && char === "'" && prev !== "\\") inSingle = !inSingle; + else if (!inSingle && !inBacktick && char === '"' && prev !== "\\") inDouble = !inDouble; + else if (!inSingle && !inDouble && char === "`" && prev !== "\\") inBacktick = !inBacktick; + else if (!inSingle && !inDouble && !inBacktick && char === "/" && line[index + 1] === "/") { + return line.slice(0, index); + } + } + + return line; +} + +export function findGitPushOffenses(text) { + const lines = text.split("\n"); + const offenses = []; + + for (let index = 0; index < lines.length; index += 1) { + const line = lines[index]; + const stripped = stripLineComment(line); + if (!lineMatchesGitPush(stripped)) continue; + + const previousLine = index > 0 ? lines[index - 1] : ""; + const isAllowed = line.includes(ALLOW_MARKER) || previousLine.includes(ALLOW_MARKER); + if (isAllowed) continue; + + offenses.push({ lineNumber: index + 1, line: line.trimEnd() }); + } + + return offenses; +} + +function shouldScanFile(relativePath) { + if (SKIP_FILENAME_SUFFIXES.some((suffix) => relativePath.endsWith(suffix))) return false; + const extension = path.extname(relativePath); + return SCANNABLE_EXTENSIONS.has(extension); +} + +export function collectScannableFiles(absoluteRoot, repoRoot) { + const results = []; + let stats; + try { + stats = statSync(absoluteRoot); + } catch { + return results; + } + if (!stats.isDirectory()) return results; + + const stack = [absoluteRoot]; + while (stack.length > 0) { + const current = stack.pop(); + let entries; + try { + entries = readdirSync(current, { withFileTypes: true }); + } catch { + continue; + } + for (const entry of entries) { + if (entry.isDirectory()) { + if (SKIP_DIRECTORY_NAMES.has(entry.name)) continue; + stack.push(path.join(current, entry.name)); + continue; + } + const absolute = path.join(current, entry.name); + const relative = path.relative(repoRoot, absolute).split(path.sep).join("/"); + if (shouldScanFile(relative)) results.push({ absolute, relative }); + } + } + + return results; +} + +export function runCheck({ repoRoot, scanRoots = DEFAULT_SCAN_ROOTS, log = console.log, error = console.error } = {}) { + const allOffenses = []; + + for (const scanRoot of scanRoots) { + const absoluteRoot = path.resolve(repoRoot, scanRoot); + const files = collectScannableFiles(absoluteRoot, repoRoot); + for (const file of files) { + let text; + try { + text = readFileSync(file.absolute, "utf8"); + } catch { + continue; + } + const offenses = findGitPushOffenses(text); + for (const offense of offenses) { + allOffenses.push({ relative: file.relative, ...offense }); + } + } + } + + if (allOffenses.length > 0) { + error("ERROR: `git push` (or equivalent remote-mutating git command) found in adapter/runtime code:\n"); + for (const offense of allOffenses) { + error(` ${offense.relative}:${offense.lineNumber}: ${offense.line}`); + } + error( + "\nAdapter and runtime code must not push to a git remote. The local execution-workspace cwd is the only persistence boundary between runs (see packages/adapters/AUTHORING.md and PAPA-432).", + ); + error( + `If the operator has explicitly configured a path that must push, add a \`${ALLOW_MARKER}: \` comment on the matching line or the line immediately above to opt in.`, + ); + return 1; + } + + log(` ✓ No unapproved \`git push\` invocations found in adapter/runtime code.`); + return 0; +} + +function isMainModule() { + return process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); +} + +if (isMainModule()) { + const repoRoot = process.cwd(); + process.exit(runCheck({ repoRoot })); +} diff --git a/scripts/check-no-git-push.test.mjs b/scripts/check-no-git-push.test.mjs new file mode 100644 index 00000000000..90db672160f --- /dev/null +++ b/scripts/check-no-git-push.test.mjs @@ -0,0 +1,162 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; + +import { + ALLOW_MARKER, + GIT_PUSH_PATTERN, + collectScannableFiles, + findGitPushOffenses, + runCheck, +} from "./check-no-git-push.mjs"; + +test("regex matches common git push forms", () => { + assert.ok(GIT_PUSH_PATTERN.test("git push")); + assert.ok(GIT_PUSH_PATTERN.test("GIT PUSH")); + assert.ok(GIT_PUSH_PATTERN.test("git push origin master")); + assert.ok(GIT_PUSH_PATTERN.test("git-push")); + assert.ok(GIT_PUSH_PATTERN.test("git_push")); +}); + +test("regex ignores unrelated `push` usages", () => { + assert.ok(!GIT_PUSH_PATTERN.test("args.push('git')")); + assert.ok(!GIT_PUSH_PATTERN.test("notes.push('git remote')")); + assert.ok(!GIT_PUSH_PATTERN.test("pushed")); + assert.ok(!GIT_PUSH_PATTERN.test("git fetch")); +}); + +test("findGitPushOffenses flags a bare invocation in a string", () => { + const text = `await exec("git push origin master");\n`; + const offenses = findGitPushOffenses(text); + assert.equal(offenses.length, 1); + assert.equal(offenses[0].lineNumber, 1); +}); + +test("findGitPushOffenses ignores mentions inside `//` comments", () => { + const text = `// sync-back alone — no \`git push\`, no fetch from any origin.\nconst x = 1;\n`; + assert.deepEqual(findGitPushOffenses(text), []); +}); + +test("findGitPushOffenses allows opt-in marker on the same line", () => { + const text = `await exec("git push origin master"); // ${ALLOW_MARKER}: operator-configured release mirror\n`; + assert.deepEqual(findGitPushOffenses(text), []); +}); + +test("findGitPushOffenses allows opt-in marker on the line above", () => { + const text = `// ${ALLOW_MARKER}: operator-configured release mirror\nawait exec("git push origin master");\n`; + assert.deepEqual(findGitPushOffenses(text), []); +}); + +test("findGitPushOffenses flags string-literal push even when text is split across mixed quotes", () => { + const text = "const cmd = `git push --tags`;\n"; + const offenses = findGitPushOffenses(text); + assert.equal(offenses.length, 1); +}); + +test("findGitPushOffenses flags args-array form passed to spawn/execFile", () => { + const cases = [ + `spawn("git", ["push", "origin", "main"]);\n`, + `execFile('git', ['push', '--tags']);\n`, + "execFile(`git`, [`push`, `--mirror`]);\n", + ]; + for (const text of cases) { + const offenses = findGitPushOffenses(text); + assert.equal(offenses.length, 1, `expected match for ${text}`); + } +}); + +test("findGitPushOffenses does not flag args-array form when allow marker is present", () => { + const text = `// ${ALLOW_MARKER}: release tooling adapter\nspawn("git", ["push", "origin", "main"]);\n`; + assert.deepEqual(findGitPushOffenses(text), []); +}); + +test("runCheck passes when scoped tree has no offenses", () => { + const tmpRoot = mkdtempSync(path.join(os.tmpdir(), "no-git-push-pass-")); + try { + mkdirSync(path.join(tmpRoot, "packages/adapters/sample/src"), { recursive: true }); + writeFileSync( + path.join(tmpRoot, "packages/adapters/sample/src/index.ts"), + "export const ok = 1;\n", + ); + const logs = []; + const errors = []; + const code = runCheck({ + repoRoot: tmpRoot, + scanRoots: ["packages/adapters"], + log: (msg) => logs.push(msg), + error: (msg) => errors.push(msg), + }); + assert.equal(code, 0); + assert.equal(errors.length, 0); + } finally { + rmSync(tmpRoot, { recursive: true, force: true }); + } +}); + +test("runCheck fails when scoped tree contains an unapproved git push", () => { + const tmpRoot = mkdtempSync(path.join(os.tmpdir(), "no-git-push-fail-")); + try { + mkdirSync(path.join(tmpRoot, "packages/adapters/sample/src"), { recursive: true }); + writeFileSync( + path.join(tmpRoot, "packages/adapters/sample/src/index.ts"), + "import { execSync } from 'node:child_process';\nexecSync('git push origin main');\n", + ); + const logs = []; + const errors = []; + const code = runCheck({ + repoRoot: tmpRoot, + scanRoots: ["packages/adapters"], + log: (msg) => logs.push(msg), + error: (msg) => errors.push(msg), + }); + assert.equal(code, 1); + assert.ok(errors.some((line) => line.includes("packages/adapters/sample/src/index.ts:2"))); + } finally { + rmSync(tmpRoot, { recursive: true, force: true }); + } +}); + +test("runCheck ignores opt-in marker outside the scoped tree", () => { + const tmpRoot = mkdtempSync(path.join(os.tmpdir(), "no-git-push-scope-")); + try { + mkdirSync(path.join(tmpRoot, "scripts"), { recursive: true }); + writeFileSync( + path.join(tmpRoot, "scripts/release.mjs"), + "execSync('git push origin v1.2.3');\n", + ); + const code = runCheck({ + repoRoot: tmpRoot, + scanRoots: ["packages/adapters", "server/src"], + log: () => {}, + error: () => {}, + }); + assert.equal(code, 0); + } finally { + rmSync(tmpRoot, { recursive: true, force: true }); + } +}); + +test("collectScannableFiles skips node_modules, dist, and .d.ts", () => { + const tmpRoot = mkdtempSync(path.join(os.tmpdir(), "no-git-push-collect-")); + try { + const adaptersRoot = path.join(tmpRoot, "packages/adapters/sample"); + mkdirSync(path.join(adaptersRoot, "src"), { recursive: true }); + mkdirSync(path.join(adaptersRoot, "dist"), { recursive: true }); + mkdirSync(path.join(adaptersRoot, "node_modules/pkg"), { recursive: true }); + writeFileSync(path.join(adaptersRoot, "src/index.ts"), ""); + writeFileSync(path.join(adaptersRoot, "src/types.d.ts"), ""); + writeFileSync(path.join(adaptersRoot, "dist/index.js"), ""); + writeFileSync(path.join(adaptersRoot, "node_modules/pkg/index.js"), ""); + + const files = collectScannableFiles( + path.join(tmpRoot, "packages/adapters"), + tmpRoot, + ); + const relatives = files.map((entry) => entry.relative).sort(); + assert.deepEqual(relatives, ["packages/adapters/sample/src/index.ts"]); + } finally { + rmSync(tmpRoot, { recursive: true, force: true }); + } +});