Imports without a GitHub token skip Actions variables instead of failing (#55)
GitHub answers Actions variables, secrets and environments only with a token (401 "Requires authentication" otherwise), so a token-free import of a public repository failed at the variables step, which turned the live test red. Without a token that step is now skipped with a warning, and a 401 joins 403 and 404 as "not available" wherever the importer reads admin-only data. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL
Canberk Demirci authored · GitHub committed signed1 parent 390db27commit 1d7c878a6dd7e29f99e06a4276efd8f16a776efcBrowse files
3 files changed+32−6
modifiedapps/diffinery/src/imports/service.ts+10−3View file
| @@ -175,6 +175,8 @@ | ||
| 175 | 175 | const errMsg = (e: unknown) => (e instanceof Error ? e.message.replace(/^\d{3}: /, "") : String(e)); |
| 176 | 176 | |
| 177 | 177 | /** A valid Actions variable name: GitHub's rules (letters, digits, underscore, not starting with a digit) and never the reserved GITHUB_ prefix. */ |
| 178 | +/** A GitHub answer meaning the data isn't available to this token (none, or one without admin access), not a failure. */ | |
| 179 | +const notAvailable = (e: unknown): boolean => e instanceof GitHubError && (e.status === 401 || e.status === 403 || e.status === 404); | |
| 178 | 180 | const validVarName = (name: string): boolean => /^[A-Za-z_][A-Za-z0-9_]*$/.test(name) && !/^GITHUB_/i.test(name) && name.length <= 256; |
| 179 | 181 | |
| 180 | 182 | /** |
| @@ -997,7 +999,7 @@ | ||
| 997 | 999 | try { |
| 998 | 1000 | p = await ctx.gh.get<GhProtection>(`repos/${src}/branches/${encodeURIComponent(b.name)}/protection`); |
| 999 | 1001 | } catch (e) { |
| 1000 | - if (e instanceof GitHubError && (e.status === 403 || e.status === 404)) { | |
| 1002 | + if (notAvailable(e)) { | |
| 1001 | 1003 | await this.warn(ctx.row.id, `Branch protection of ${b.name} could not be read (the GitHub token needs admin access to the repository); set it up under Settings → Branch protection.`); |
| 1002 | 1004 | continue; |
| 1003 | 1005 | } |
| @@ -1030,12 +1032,17 @@ | ||
| 1030 | 1032 | if (!ctx.req.options.variables) return { done: true, skipped: true, message: "Not selected" }; |
| 1031 | 1033 | const may = await this.mayManageVariables(ctx); |
| 1032 | 1034 | if (!may.ok) return { done: true, skipped: true, message: may.reason }; |
| 1035 | + // GitHub answers these admin endpoints only with a token (401 "Requires authentication" otherwise). | |
| 1036 | + if (!ctx.token) { | |
| 1037 | + await this.warn(ctx.row.id, "Actions variables, secret names and environments were not imported: reading them needs a GitHub token with admin access to the repository. Add them under Settings → Secrets and variables."); | |
| 1038 | + return { done: true, skipped: true, message: "skipped (needs a GitHub token)" }; | |
| 1039 | + } | |
| 1033 | 1040 | const src = ctx.row.source; |
| 1034 | 1041 | const read = async <T>(path: string, what: string): Promise<T[]> => { |
| 1035 | 1042 | try { |
| 1036 | 1043 | return await ctx.gh.all<T>(path, 1000); |
| 1037 | 1044 | } catch (e) { |
| 1038 | - if (e instanceof GitHubError && (e.status === 403 || e.status === 404)) { | |
| 1045 | + if (notAvailable(e)) { | |
| 1039 | 1046 | await this.warn(ctx.row.id, `Actions ${what} could not be read (the GitHub token needs admin access, or "Variables"/"Secrets" read permission for fine-grained tokens).`); |
| 1040 | 1047 | return []; |
| 1041 | 1048 | } |
| @@ -1234,7 +1241,7 @@ | ||
| 1234 | 1241 | return await ctx.gh.all<T>(path, 1000); |
| 1235 | 1242 | } catch (e) { |
| 1236 | 1243 | if (e instanceof RateLimited) throw e; |
| 1237 | - if (e instanceof GitHubError && (e.status === 403 || e.status === 404)) { | |
| 1244 | + if (notAvailable(e)) { | |
| 1238 | 1245 | await this.warn(id, `Organization ${what} could not be read (the GitHub token needs organization admin access).`); |
| 1239 | 1246 | return []; |
| 1240 | 1247 | } |
modifiedapps/diffinery/test/e2e/e2e.test.mjs+14−1View file
| @@ -2300,7 +2300,7 @@ | ||
| 2300 | 2300 | "GET /repos/octo/lib/actions/secrets": { secrets: [] }, |
| 2301 | 2301 | "GET /repos/octo/lib/releases": empty, |
| 2302 | 2302 | }; |
| 2303 | - const gh = fakeGitHub({ root: ghRoot, token: GH_TOKEN, routes, lfs: new Map([[fx.asset.oid, fx.asset.data]]) }); | |
| 2303 | + const gh = fakeGitHub({ root: ghRoot, token: GH_TOKEN, routes, lfs: new Map([[fx.asset.oid, fx.asset.data]]), anonymous: ["octo/lib"] }); | |
| 2304 | 2304 | ghBase = await gh.listen(); |
| 2305 | 2305 | const apiUrl = `${ghBase}/api/v3`; |
| 2306 | 2306 | |
| @@ -2509,6 +2509,19 @@ | ||
| 2509 | 2509 | assert.equal((await ok("GET", "/repos/acme-imports/lib/issues/1")).title, "Lib issue"); |
| 2510 | 2510 | assert.equal((await ok("GET", "/repos/acme-imports/app/pulls/2")).merged, true); |
| 2511 | 2511 | |
| 2512 | + // ------------------------------------------------------------ without a token: everything public comes across, variables are skipped | |
| 2513 | + // GitHub answers Actions variables, secrets and environments only with a token (401 otherwise). | |
| 2514 | + const anon = await ok("POST", "/diffinery/imports", { source: "octo/lib", owner, name: "anon-lib", github_api_url: apiUrl, options: { lfs: false } }); | |
| 2515 | + assert.equal((await transfer(anon.id)).status, 0); | |
| 2516 | + const anonDone = await finished(anon.id); | |
| 2517 | + assert.equal(anonDone.status, "succeeded", JSON.stringify(anonDone, null, 2)); | |
| 2518 | + const anonVars = anonDone.steps.find((st) => st.name === "variables"); | |
| 2519 | + assert.equal(anonVars.status, "skipped", JSON.stringify(anonDone.steps)); | |
| 2520 | + assert.match(anonVars.message, /needs a GitHub token/); | |
| 2521 | + assert.ok(anonDone.warnings.some((w) => /variables, secret names and environments were not imported/.test(w)), JSON.stringify(anonDone.warnings)); | |
| 2522 | + assert.equal((await ok("GET", `/repos/${owner}/anon-lib/issues/1`)).title, "Lib issue"); | |
| 2523 | + assert.equal((await ok("GET", `/repos/${owner}/anon-lib`)).default_branch, "trunk"); | |
| 2524 | + | |
| 2512 | 2525 | // ------------------------------------------------------------ an agent's organization import cannot touch variables (H1) |
| 2513 | 2526 | const agentBulk = await api("POST", "/diffinery/imports", { source_owner: "octo", owner: "acme-imports", github_token: GH_TOKEN, github_api_url: apiUrl, concurrency: 1, options: { lfs: false } }, bot); |
| 2514 | 2527 | assert.equal(agentBulk.status, 201, JSON.stringify(agentBulk.json)); |
modifiedapps/diffinery/test/e2e/fake-github.mjs+8−2View file
| @@ -12,8 +12,11 @@ | ||
| 12 | 12 | * @param {string} o.token the token every request must carry |
| 13 | 13 | * @param {Record<string, unknown>} o.routes "GET /repos/o/r/labels" → JSON (arrays are paginated) |
| 14 | 14 | * @param {Map<string, Buffer>} o.lfs LFS objects by oid |
| 15 | + * @param {string[]} [o.anonymous] repositories ("owner/name") anyone may read without a token, like public | |
| 16 | + * ones on GitHub; their admin endpoints (Actions variables and secrets, environments, branch protection) | |
| 17 | + * still answer 401 "Requires authentication" without one | |
| 15 | 18 | */ |
| 16 | -export function fakeGitHub({ root, token, routes, lfs }) { | |
| 19 | +export function fakeGitHub({ root, token, routes, lfs, anonymous = [] }) { | |
| 17 | 20 | const requests = []; |
| 18 | 21 | let base; |
| 19 | 22 | const authorized = (req) => { |
| @@ -34,7 +37,10 @@ | ||
| 34 | 37 | requests.push({ method: req.method, path: url.pathname, query: url.search }); |
| 35 | 38 | // Public, unauthenticated GitHub endpoints: a user profile and their public gists (used by gist-proof account linking). |
| 36 | 39 | const publicGet = req.method === "GET" && /^\/api\/v3\/(users\/[^/]+(?:\/gists)?|gists\/[^/]+)$/.test(url.pathname); |
| 37 | - if (!publicGet && !authorized(req)) return json(res, 401, { message: "Bad credentials" }, { "www-authenticate": 'Basic realm="GitHub"' }); | |
| 40 | + const repoOf = /^\/(?:api\/v3\/repos\/)?([^/]+\/[^/]+?)(?:\.git)?(?:\/|$)/.exec(url.pathname)?.[1]; | |
| 41 | + const anonymousRead = !req.headers.authorization && anonymous.includes(repoOf) && (req.method === "GET" || /\/git-upload-pack$/.test(url.pathname)); | |
| 42 | + if (anonymousRead && /\/(actions|environments)(\/|$)|\/protection$/.test(url.pathname)) return json(res, 401, { message: "Requires authentication" }); | |
| 43 | + if (!publicGet && !anonymousRead && !authorized(req)) return json(res, 401, { message: "Bad credentials" }, { "www-authenticate": 'Basic realm="GitHub"' }); | |
| 38 | 44 | |
| 39 | 45 | // ---------------------------------------------------------------- Git LFS |
| 40 | 46 | const lfsBatch = /^\/([^/]+)\/([^/]+?)(?:\.git)?\/info\/lfs\/objects\/batch$/.exec(url.pathname); |