mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
ci-audit: adversarial-pass fixes (schedule guard, issue pages, references escaped)
- The Saturday job runs on every schedule event, so an empty repository.default_branch on schedule payloads cannot skip it. - findIssue pages through up to 500 open bot issues. - Names also break #N, owner/repo#N references and bare URLs. - Docs say a partial audit opens the issue only when none is open. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
7bb277840b
commit
7c8b53fb72
5 files changed
+23
-15
No files matched your search
+2
-1
@@ -83,7 +83,8 @@ Turn it off with `gstack-config set ci_audit_ship off`.
|
||||
`lib/ci-audit/templates/ci-audit-weekly.yml` runs the full report every Saturday at 15:00 UTC
|
||||
and keeps one open "Weekly CI audit" issue current. A complete audit creates or replaces
|
||||
the issue, or closes it when nothing is left to fix. A partial or failed audit comments
|
||||
on the issue and keeps the last complete findings. The issue is matched by its author
|
||||
on the issue and keeps the last complete findings; with no issue open, a partial audit
|
||||
that has findings opens one, with its `Partial:` line saying what was cut. The issue is matched by its author
|
||||
(`github-actions[bot]`, the workflow's own token), its title and a marker line, so an
|
||||
issue anyone else opened, even with the same title and marker, is left alone. It reads
|
||||
with the workflow's `GITHUB_TOKEN` (1,000 calls an hour), so it runs with
|
||||
|
||||
+10
-5
@@ -51,11 +51,16 @@ export const ISSUE_AUTHOR = 'github-actions[bot]';
|
||||
* The open issue this audit owns: opened by the workflow's own token, with its title and marker.
|
||||
* Anyone can copy the title and marker, so an issue another account opened is never touched.
|
||||
*/
|
||||
export async function findIssue(client: ApiClient, repo: string): Promise<Existing | null> {
|
||||
const res = await client.get(`/repos/${repo}/issues?state=open&creator=${encodeURIComponent(ISSUE_AUTHOR)}&per_page=100&sort=created&direction=desc`);
|
||||
if (res.status !== 200) throw new AuditError('ci_audit_api_failed', `issues list returned HTTP ${res.status}`, 'now');
|
||||
const hit = ((res.json as any[]) ?? []).find(i => !i.pull_request && i.user?.login === ISSUE_AUTHOR && i.title === ISSUE_TITLE && String(i.body ?? '').includes(ISSUE_MARKER));
|
||||
return hit ? { number: Number(hit.number), body: String(hit.body) } : null;
|
||||
export async function findIssue(client: ApiClient, repo: string, maxPages = 5): Promise<Existing | null> {
|
||||
for (let page = 1; page <= maxPages; page++) {
|
||||
const res = await client.get(`/repos/${repo}/issues?state=open&creator=${encodeURIComponent(ISSUE_AUTHOR)}&per_page=100&sort=created&direction=desc&page=${page}`);
|
||||
if (res.status !== 200) throw new AuditError('ci_audit_api_failed', `issues list returned HTTP ${res.status}`, 'now');
|
||||
const rows = (res.json as any[]) ?? [];
|
||||
const hit = rows.find(i => !i.pull_request && i.user?.login === ISSUE_AUTHOR && i.title === ISSUE_TITLE && String(i.body ?? '').includes(ISSUE_MARKER));
|
||||
if (hit) return { number: Number(hit.number), body: String(hit.body) };
|
||||
if (rows.length < 100) return null;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export type GhWrite = (method: 'POST' | 'PATCH', apiPath: string, body: Record<string, unknown>) => Promise<number>;
|
||||
|
||||
@@ -59,8 +59,8 @@ export function buildReport(data: AuditData, m: Measures, ls: Lesson[], st: Stat
|
||||
const t = (s: number | null) => (s == null ? '-' : s >= 120 ? `${(s / 60).toFixed(1)}m` : `${Math.round(s)}s`);
|
||||
const STATE = { pass: 'pass', finding: 'FINDING', unknown: 'unknown', not_applicable: 'n/a' } as const;
|
||||
|
||||
/** Job and workflow names come from workflow files a fork PR controls: no table breaks, links, HTML or mentions. */
|
||||
const esc = (s: string) => String(s).replace(/[\\|<>[\]`]/g, c => `\\${c}`).replace(/@/g, '@\u200b');
|
||||
/** Job and workflow names come from workflow files a fork PR controls: no table breaks, links, HTML, mentions or issue references. */
|
||||
const esc = (s: string) => String(s).replace(/[\\|<>[\]`]/g, c => `\\${c}`).replace(/[@#]/g, c => `${c}\u200b`).replace(/:\/\//g, ':\u200b//');
|
||||
|
||||
export function renderMarkdown(r: AuditReport, maxJobs = 12): string {
|
||||
const out: string[] = [];
|
||||
|
||||
@@ -22,7 +22,7 @@ concurrency:
|
||||
|
||||
jobs:
|
||||
audit:
|
||||
if: github.ref_name == github.event.repository.default_branch
|
||||
if: github.event_name == 'schedule' || github.ref_name == github.event.repository.default_branch
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 20
|
||||
env:
|
||||
|
||||
@@ -267,21 +267,23 @@ describe('weekly issue', () => {
|
||||
|
||||
test('job names from a fork PR cannot break the tables or mention anyone', () => {
|
||||
const t = { p50: 1, p90: 2 };
|
||||
const body = planIssue({ exit: 0, report: { ...withFixes, jobs: [{ key: 'x | [y](https://e.example) @team <b>', runner: 'ubuntu-latest', n: 1, wait: t, setup: t, run: t, billedMinutesPerWeek: 1, usdPerMonth: 0 }] } as unknown as AuditReport }, null) as { body: string };
|
||||
const body = planIssue({ exit: 0, report: { ...withFixes, jobs: [{ key: 'x | [y](https://e.example) @team <b> o/r#1', runner: 'ubuntu-latest', n: 1, wait: t, setup: t, run: t, billedMinutesPerWeek: 1, usdPerMonth: 0 }] } as unknown as AuditReport }, null) as { body: string };
|
||||
const row = body.body.split('\n').find(l => l.includes('e.example'))!;
|
||||
expect(row).toBe('| x \\| \\[y\\](https://e.example) @\u200bteam \\<b\\> | ubuntu-latest | 1 | 1s/2s | 1s | 1s | 1 | 0 |');
|
||||
expect(row).toBe('| x \\| \\[y\\](https:\u200b//e.example) @\u200bteam \\<b\\> o/r#\u200b1 | ubuntu-latest | 1 | 1s/2s | 1s | 1s | 1 | 0 |');
|
||||
});
|
||||
|
||||
test('finds only the workflow bot\'s own issue, past 100 open issues and an outsider\'s copy', async () => {
|
||||
test('finds only the workflow bot\'s own issue, past 100 open issues, 100 newer bot issues and an outsider\'s copy', async () => {
|
||||
const issue = (number: number, login: string, title = 'other', body = '') => ({ number, title, body, user: { login } });
|
||||
const open = [issue(1, 'outsider', ISSUE_TITLE, `${ISSUE_MARKER}\nfake findings`),
|
||||
...Array.from({ length: 150 }, (_, i) => issue(i + 2, 'person')),
|
||||
issue(152, 'github-actions[bot]', ISSUE_TITLE, `${ISSUE_MARKER}\nreal findings`)];
|
||||
issue(152, 'github-actions[bot]', ISSUE_TITLE, `${ISSUE_MARKER}\nreal findings`),
|
||||
...Array.from({ length: 120 }, (_, i) => issue(i + 153, 'github-actions[bot]', 'Nightly flake report'))];
|
||||
const client = { text: async () => null, get: async (apiPath: string) => {
|
||||
const q = new URL(apiPath, 'https://api.github.com').searchParams;
|
||||
const rows = open.filter(i => !q.get('creator') || i.user.login === q.get('creator'));
|
||||
if (q.get('direction') !== 'asc') rows.reverse();
|
||||
return { status: 200, json: rows.slice(0, Number(q.get('per_page') ?? 30)), headers: {} };
|
||||
const per = Number(q.get('per_page') ?? 30), page = Number(q.get('page') ?? 1);
|
||||
return { status: 200, json: rows.slice((page - 1) * per, page * per), headers: {} };
|
||||
} };
|
||||
expect(await findIssue(client as never, 'o/n')).toEqual({ number: 152, body: `${ISSUE_MARKER}\nreal findings` });
|
||||
});
|
||||
@@ -295,7 +297,7 @@ describe('Saturday workflow template', () => {
|
||||
expect(doc.on.schedule).toEqual([{ cron: '0 15 * * 6' }]);
|
||||
expect('workflow_dispatch' in doc.on).toBe(true);
|
||||
expect(doc.permissions).toEqual({ actions: 'read', contents: 'read', issues: 'write' });
|
||||
expect(doc.jobs.audit.if).toBe('github.ref_name == github.event.repository.default_branch');
|
||||
expect(doc.jobs.audit.if).toBe("github.event_name == 'schedule' || github.ref_name == github.event.repository.default_branch");
|
||||
});
|
||||
|
||||
test('audits within the GITHUB_TOKEN hourly limit and always publishes the outcome', () => {
|
||||
|
||||
Reference in new issue
Block a user