Commit8ae1b1f
feat(onboarding): bulk import now secures and scans what it imported
feat(onboarding): bulk import now secures and scans what it imported
POST /import/bulk cloned repositories, inserted rows, and stopped. A team
migrating an org therefore landed on a set of bare repositories — no branch
protection, no labels, no CODEOWNERS, no gate settings — and no evidence the
migration was worth doing. The product's entire pitch, that it finds problems
in your code, was something they had to go and discover for themselves one
repo at a time.
After a real import each newly cloned repo now gets:
bootstrapRepository() green defaults — gates on, branch protection,
labels, CODEOWNERS
runSecretAndSecurityScan() the actual proof — committed secrets and
security findings in the code they just brought
over
...and the results page reports it: how many repos were secured, how many
committed secrets were found, and a per-repo table linking straight to
/:owner/:repo/security. When nothing is found it says so and points out that
gates are now live, which is also a result.
No new engines. bootstrapRepository, runSecretAndSecurityScan and
mapWithConcurrency all existed; the gap was that nothing connected them to
the import path.
Design decisions worth stating:
- Only `status === "success"` repos are onboarded. "skipped-exists" means the
repo was already on the platform with settings its owner chose, and
re-bootstrapping would fight those. (The typechecker caught my first
attempt using a status value that does not exist.)
- Fault isolation is the property that matters: a 40-repo migration must not
be reported as failed because the scanner threw on one of them. Each repo's
outcome is captured independently, a throw becomes a per-repo `error`
field, and a bootstrap failure still lets that repo's scan run — the scan
is the value, so it must not be skipped as collateral. Tested with injected
failures rather than asserted in a comment.
- Concurrency is bounded. Each scan is a tree walk plus blob reads; an
unbounded Promise.all over a 100-repo org would open 100 concurrent walks
against one disk and one connection pool.
- The whole block is wrapped so a failure cannot break the results page. The
repositories are already cloned by then and that must be reported.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>3 files changed+428−08ae1b1f4fceb1d8358074775428c62f3d5053984
3 changed files+428−0
Addedsrc/__tests__/migration-onboarding.test.ts+149−0View fileUnifiedSplit
@@ -0,0 +1,149 @@
1/**
2 * Post-import onboarding for a team migrating from GitHub.
3 *
4 * POST /import/bulk cloned repositories and stopped. A team that migrated an
5 * org landed on bare repos — no branch protection, no labels, no gate
6 * settings — and no evidence the migration was worth doing. The platform's
7 * whole pitch (it finds problems in your code) was something they had to go
8 * and discover for themselves, one repo at a time.
9 *
10 * runMigrationOnboarding bootstraps green defaults and scans the imported
11 * code, returning a report the results page renders.
12 *
13 * The property that matters most here is FAULT ISOLATION: a migration that
14 * imported 40 repositories must not be reported as a failure because the
15 * scanner threw on one of them. These use injected failures to prove that.
16 */
17
18import { describe, expect, it, mock, afterEach } from "bun:test";
19import { readFileSync } from "fs";
20
21const SRC = readFileSync("src/lib/migration-onboarding.ts", "utf8");
22const ROUTE = readFileSync("src/routes/import-bulk.tsx", "utf8");
23
24describe("wiring", () => {
25 it("import-bulk runs onboarding after a real import", () => {
26 expect(ROUTE).toContain("runMigrationOnboarding");
27 });
28
29 it("only newly cloned repos are onboarded", () => {
30 // "skipped-exists" repos were already on the platform and have settings
31 // their owner chose; re-bootstrapping would fight those.
32 expect(ROUTE).toContain('r.status === "success"');
33 expect(ROUTE).not.toContain('r.status === "imported"');
34 });
35
36 it("a failure in onboarding cannot fail the completed migration", () => {
37 // lastIndexOf: there is an earlier `return c.html(` for the dry-run
38 // preview, which would slice to an empty string and pass vacuously.
39 const block = ROUTE.slice(
40 ROUTE.indexOf("const importedNames"),
41 ROUTE.lastIndexOf("return c.html(")
42 );
43 expect(block.length).toBeGreaterThan(100);
44 expect(block).toContain("try {");
45 expect(block).toContain("catch");
46 // The repositories are already cloned at this point; the results page
47 // must still render.
48 expect(block).not.toMatch(/throw\s/);
49 });
50
51 it("skips the welcome issue on imported repos", () => {
52 // An imported repo arrives with real history and real issues. A
53 // "welcome" issue on top of 400 migrated ones is noise.
54 expect(SRC).toContain("skipWelcomeIssue: true");
55 });
56
57 it("bounds concurrency rather than scanning every repo at once", () => {
58 // Each scan is a tree walk plus blob reads; an unbounded Promise.all over
59 // a 100-repo org opens 100 concurrent walks against one disk and pool.
60 expect(SRC).toContain("mapWithConcurrency");
61 expect(SRC).toContain("DB_FANOUT_LIMIT");
62 });
63});
64
65describe("fault isolation", () => {
66 afterEach(() => {
67 mock.restore();
68 });
69
70 it("one repo's scan failure does not sink the others", async () => {
71 mock.module("../lib/repo-bootstrap", () => ({
72 bootstrapRepository: async () => ({
73 settingsCreated: true,
74 protectionCreated: true,
75 labelsCreated: 5,
76 }),
77 }));
78 mock.module("../lib/gate", () => ({
79 runSecretAndSecurityScan: async (_o: string, repo: string) => {
80 if (repo === "explodes") throw new Error("scanner blew up");
81 return {
82 secretResult: { name: "Secrets", passed: true, details: "clean" },
83 securityResult: { name: "Security", passed: true, details: "clean" },
84 secrets: repo === "leaky" ? [{ a: 1 }, { b: 2 }] : [],
85 securityIssues: [],
86 };
87 },
88 }));
89
90 const { runMigrationOnboarding } = await import("../lib/migration-onboarding");
91 const report = await runMigrationOnboarding(
92 [
93 { id: "1", owner: "acme", name: "ok" },
94 { id: "2", owner: "acme", name: "explodes" },
95 { id: "3", owner: "acme", name: "leaky" },
96 ],
97 "user-1"
98 );
99
100 expect(report.totalRepos).toBe(3);
101 // The thrower is reported, not thrown.
102 const bad = report.repos.find((r) => r.name === "explodes")!;
103 expect(bad.error).toContain("scanner blew up");
104 // ...and it still got its green defaults.
105 expect(bad.bootstrapped).toBe(true);
106 // The others are unaffected and their findings are counted.
107 expect(report.totalSecrets).toBe(2);
108 expect(report.reposWithFindings).toBe(1);
109 expect(report.reposBootstrapped).toBe(3);
110 });
111
112 it("a bootstrap failure still lets the scan run", async () => {
113 mock.module("../lib/repo-bootstrap", () => ({
114 bootstrapRepository: async () => {
115 throw new Error("no protection for you");
116 },
117 }));
118 mock.module("../lib/gate", () => ({
119 runSecretAndSecurityScan: async () => ({
120 secretResult: { name: "Secrets", passed: false, details: "1 secret" },
121 securityResult: { name: "Security", passed: true, details: "clean" },
122 secrets: [{ a: 1 }],
123 securityIssues: [],
124 }),
125 }));
126
127 const { runMigrationOnboarding } = await import("../lib/migration-onboarding");
128 const report = await runMigrationOnboarding(
129 [{ id: "1", owner: "acme", name: "repo" }],
130 "user-1"
131 );
132
133 const r = report.repos[0];
134 expect(r.bootstrapped).toBe(false);
135 expect(r.error).toContain("bootstrap");
136 // The scan is the value proposition — it must not be skipped because
137 // the bootstrap step failed.
138 expect(r.secretsFound).toBe(1);
139 expect(report.totalSecrets).toBe(1);
140 });
141
142 it("returns an empty report for an empty import rather than throwing", async () => {
143 const { runMigrationOnboarding } = await import("../lib/migration-onboarding");
144 const report = await runMigrationOnboarding([], "user-1");
145 expect(report.totalRepos).toBe(0);
146 expect(report.totalSecrets).toBe(0);
147 expect(report.reposWithFindings).toBe(0);
148 });
149});
Addedsrc/lib/migration-onboarding.ts+141−0View fileUnifiedSplit
@@ -0,0 +1,141 @@
1/**
2 * Post-import onboarding for a team migrating from GitHub.
3 *
4 * `POST /import/bulk` cloned repositories and inserted rows, and stopped
5 * there. A team that migrated an org therefore landed on a set of bare
6 * repositories with no branch protection, no labels, no CODEOWNERS, no gate
7 * settings, and no evidence that any of it was worth doing. The platform's
8 * entire value proposition — that it finds problems in your code — was
9 * something they had to go and discover for themselves, one repo at a time.
10 *
11 * This runs the work for them at import time and returns a report:
12 *
13 * 1. bootstrapRepository() green defaults: gates on, branch
14 * protection, labels, CODEOWNERS.
15 * 2. runSecretAndSecurityScan() the actual proof — committed secrets and
16 * security findings in the code they just
17 * brought over.
18 *
19 * Everything here is best-effort by design. A migration that imported 40
20 * repositories must not be reported as a failure because the scanner choked
21 * on one of them, so each repo's outcome is captured independently and a
22 * thrown error becomes a per-repo `error` field rather than a rejected
23 * promise.
24 *
25 * Concurrency is bounded (DB_FANOUT_LIMIT). Scanning is IO-heavy — a tree
26 * walk plus blob reads per repo — and an unbounded Promise.all over a
27 * 100-repo org would open 100 concurrent walks against the same disk and
28 * connection pool.
29 */
30
31import { mapWithConcurrency, DB_FANOUT_LIMIT } from "./concurrency";
32
33/** One repository's post-import outcome. */
34export interface MigrationRepoReport {
35 owner: string;
36 name: string;
37 /** Green-default bootstrap applied (settings, protection, labels). */
38 bootstrapped: boolean;
39 /** Number of branch-protection rules / labels created, for the summary. */
40 labelsCreated: number;
41 protectionCreated: boolean;
42 /** Committed secrets found in the imported code. */
43 secretsFound: number;
44 /** Security findings (vulnerable patterns) in the imported code. */
45 securityIssues: number;
46 /** Human-readable scan detail, already scrubbed by the scanner. */
47 scanDetail: string;
48 /** Set when this repo's onboarding failed; the others still ran. */
49 error?: string;
50}
51
52export interface MigrationReport {
53 repos: MigrationRepoReport[];
54 /** Totals, precomputed so the view stays markup. */
55 totalRepos: number;
56 totalSecrets: number;
57 totalSecurityIssues: number;
58 reposBootstrapped: number;
59 reposWithFindings: number;
60}
61
62/**
63 * Bootstrap and scan every freshly imported repository.
64 *
65 * `repos` is the subset of a bulk import that actually landed — callers pass
66 * only successful imports, since there is nothing to scan otherwise.
67 */
68export async function runMigrationOnboarding(
69 repos: Array<{ id: string; owner: string; name: string; defaultBranch?: string | null }>,
70 ownerUserId: string
71): Promise<MigrationReport> {
72 const results = await mapWithConcurrency(
73 repos,
74 DB_FANOUT_LIMIT,
75 async (repo): Promise<MigrationRepoReport> => {
76 const base: MigrationRepoReport = {
77 owner: repo.owner,
78 name: repo.name,
79 bootstrapped: false,
80 labelsCreated: 0,
81 protectionCreated: false,
82 secretsFound: 0,
83 securityIssues: 0,
84 scanDetail: "",
85 };
86
87 const branch = repo.defaultBranch || "main";
88
89 // 1. Green defaults. Skips the welcome issue — an imported repo has
90 // real history and real issues; a "welcome" issue on top of 400
91 // migrated ones is noise, not onboarding.
92 try {
93 const { bootstrapRepository } = await import("./repo-bootstrap");
94 const r = await bootstrapRepository({
95 repositoryId: repo.id,
96 ownerUserId,
97 defaultBranch: branch,
98 skipWelcomeIssue: true,
99 });
100 base.bootstrapped = true;
101 base.labelsCreated = r.labelsCreated ?? 0;
102 base.protectionCreated = r.protectionCreated ?? false;
103 } catch (err) {
104 base.error = `bootstrap: ${err instanceof Error ? err.message : String(err)}`;
105 }
106
107 // 2. The proof. Scan the code they just brought over.
108 try {
109 const { runSecretAndSecurityScan } = await import("./gate");
110 const scan = await runSecretAndSecurityScan(
111 repo.owner,
112 repo.name,
113 `refs/heads/${branch}`,
114 branch,
115 { scanSecrets: true, scanSecurity: true }
116 );
117 base.secretsFound = scan.secrets.length;
118 base.securityIssues = scan.securityIssues.length;
119 base.scanDetail = [scan.secretResult.details, scan.securityResult.details]
120 .filter(Boolean)
121 .join(" · ");
122 } catch (err) {
123 const msg = `scan: ${err instanceof Error ? err.message : String(err)}`;
124 base.error = base.error ? `${base.error}; ${msg}` : msg;
125 }
126
127 return base;
128 }
129 );
130
131 return {
132 repos: results,
133 totalRepos: results.length,
134 totalSecrets: results.reduce((n, r) => n + r.secretsFound, 0),
135 totalSecurityIssues: results.reduce((n, r) => n + r.securityIssues, 0),
136 reposBootstrapped: results.filter((r) => r.bootstrapped).length,
137 reposWithFindings: results.filter(
138 (r) => r.secretsFound > 0 || r.securityIssues > 0
139 ).length,
140 };
141}
Modifiedsrc/routes/import-bulk.tsx+138−0View fileUnifiedSplit
@@ -267,6 +267,29 @@ const importBulkStyles = `
267267 color: var(--text-muted);
268268 }
269269
270 /* Post-import onboarding report. Scoped like the rest of this file so it
271 cannot bleed into the shared layout. */
272 .import-bulk-migration {
273 margin-top: var(--space-6);
274 padding-top: var(--space-5);
275 border-top: 1px dashed var(--border-strong);
276 }
277 .import-bulk-migration-title {
278 font-family: var(--font-display);
279 font-size: 20px;
280 font-weight: 700;
281 letter-spacing: -0.02em;
282 margin: 0 0 6px;
283 color: var(--text-strong);
284 }
285 .import-bulk-migration-sub {
286 margin: 0 0 var(--space-4);
287 font-size: 14px;
288 color: var(--text-muted);
289 line-height: 1.55;
290 max-width: 68ch;
291 }
292
270293 .import-bulk-actions {
271294 display: flex;
272295 gap: 8px;
@@ -899,6 +922,59 @@ importBulkRoutes.post("/import/bulk", requireAuth, async (c) => {
899922 {} as Record<string, number>
900923 );
901924
925 // Post-import onboarding. Until now a bulk import stopped at "cloned": a
926 // team that migrated an org got bare repositories with no branch
927 // protection, no labels, no gate settings, and no evidence any of it was
928 // worth doing. Bootstrap green defaults and scan what they just brought
929 // over, so the results page shows findings rather than a list of names.
930 //
931 // ImportOneRepoResult deliberately carries only status/name/notes, so the
932 // rows are looked up here rather than widening that contract for every
933 // caller of importOneRepo.
934 // Only newly cloned repos. "skipped-exists" means the repo was already on
935 // the platform, so it has been through this once already — re-bootstrapping
936 // would fight whatever settings its owner has since chosen.
937 const importedNames = results
938 .filter((r) => r.status === "success")
939 .map((r) => r.name);
940
941 let migration: import("../lib/migration-onboarding").MigrationReport | null = null;
942 if (importedNames.length > 0) {
943 try {
944 const { inArray, and: andOp, eq: eqOp } = await import("drizzle-orm");
945 const { db } = await import("../db");
946 const { repositories } = await import("../db/schema");
947 const rows = await db
948 .select({
949 id: repositories.id,
950 name: repositories.name,
951 defaultBranch: repositories.defaultBranch,
952 })
953 .from(repositories)
954 .where(
955 andOp(
956 eqOp(repositories.ownerId, user.id),
957 inArray(repositories.name, importedNames)
958 )
959 );
960
961 const { runMigrationOnboarding } = await import("../lib/migration-onboarding");
962 migration = await runMigrationOnboarding(
963 rows.map((r) => ({
964 id: r.id,
965 owner: user.username,
966 name: r.name,
967 defaultBranch: r.defaultBranch,
968 })),
969 user.id
970 );
971 } catch (err) {
972 // Never fail a completed migration because the follow-up work threw —
973 // the repositories are already imported and that must be reported.
974 console.error("[import-bulk] migration onboarding failed:", err);
975 }
976 }
977
902978 return c.html(
903979 <Layout title="Bulk import results" user={user}>
904980 <style dangerouslySetInnerHTML={{ __html: importBulkStyles }} />
@@ -937,6 +1013,68 @@ importBulkRoutes.post("/import/bulk", requireAuth, async (c) => {
9371013
9381014 <ResultsTable rows={results} />
9391015
1016 {migration && migration.totalRepos > 0 && (
1017 <div class="import-bulk-migration">
1018 <h2 class="import-bulk-migration-title">
1019 What we did with them
1020 </h2>
1021 <p class="import-bulk-migration-sub">
1022 Branch protection, labels and gates were applied to every
1023 repository, then we scanned the code you just brought over. You
1024 did not have to configure anything.
1025 </p>
1026
1027 <div class="import-bulk-summary">
1028 <span class="import-bulk-summary-stat">
1029 <span class="num">{migration.reposBootstrapped}</span> secured
1030 with green defaults
1031 </span>
1032 <span class="import-bulk-summary-stat">
1033 <span class="num">{migration.totalSecrets}</span> committed
1034 secret{migration.totalSecrets === 1 ? "" : "s"} found
1035 </span>
1036 <span class="import-bulk-summary-stat">
1037 <span class="num">{migration.totalSecurityIssues}</span>{" "}
1038 security finding
1039 {migration.totalSecurityIssues === 1 ? "" : "s"}
1040 </span>
1041 </div>
1042
1043 {migration.reposWithFindings > 0 ? (
1044 <table class="import-bulk-table">
1045 <thead>
1046 <tr>
1047 <th>Repository</th>
1048 <th>Secrets</th>
1049 <th>Security</th>
1050 <th>Detail</th>
1051 </tr>
1052 </thead>
1053 <tbody>
1054 {migration.repos
1055 .filter((r) => r.secretsFound > 0 || r.securityIssues > 0)
1056 .map((r) => (
1057 <tr>
1058 <td>
1059 <a href={`/${r.owner}/${r.name}/security`}>{r.name}</a>
1060 </td>
1061 <td>{r.secretsFound}</td>
1062 <td>{r.securityIssues}</td>
1063 <td class="import-bulk-note">{r.scanDetail}</td>
1064 </tr>
1065 ))}
1066 </tbody>
1067 </table>
1068 ) : (
1069 <p class="import-bulk-note">
1070 No committed secrets or security findings in the imported code.
1071 Gates are now active on every repository, so anything new gets
1072 caught at push time.
1073 </p>
1074 )}
1075 </div>
1076 )}
1077
9401078 <div class="import-bulk-actions">
9411079 <a href={`/${user.username}`} class="btn btn-primary">
9421080 View my repositories
9431081