mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
ci(adapters): reject git push in adapter/runtime code (PAPA-442)
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: <reason>` 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
9f314d97c9
commit
1195160377
4 files changed
+360
-1
No files matched your search
@@ -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
|
||||
|
||||
|
||||
+3
-1
@@ -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",
|
||||
|
||||
@@ -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: <reason>` 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}: <reason>\` 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 }));
|
||||
}
|
||||
@@ -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 });
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user