diff --git a/.changeset/ingest-junk-filters.md b/.changeset/ingest-junk-filters.md new file mode 100644 index 00000000..01bedaba --- /dev/null +++ b/.changeset/ingest-junk-filters.md @@ -0,0 +1,5 @@ +--- +"@buildinternet/uploads": minor +--- + +GitHub attachment import is on by default for linked repos, with junk filters: attachments authored by `[bot]` accounts and images under 200px on either side are skipped. A new `.uploads.yml` key, `ingestBotAttachments: true`, re-admits bot media on the webhook path; `ingestGithubAttachments: false` turns importing off per repo or per workspace as before. diff --git a/apps/api/src/github-ingest.test.ts b/apps/api/src/github-ingest.test.ts index c9690943..a7dd2665 100644 --- a/apps/api/src/github-ingest.test.ts +++ b/apps/api/src/github-ingest.test.ts @@ -27,6 +27,7 @@ import type { WorkspaceRecord } from "./workspace"; import { FakeKv } from "../test/fake-kv"; import { GITHUB_APP_CFG_ENV } from "../test/github-app-env"; import { fakeFetch, pngRoute, withGlobalFetch } from "../test/helpers/github-fetch-fakes"; +import { gifOf } from "../test/helpers/image-fixtures"; import { UsageFakeD1 } from "../test/usage-fake-d1"; const REPO = "acme/app"; @@ -336,6 +337,90 @@ describe("reconcileIngestSource", () => { expect(summary.skipped).toEqual([{ url: ASSET_URL, reason: "unsupported_media_type" }]); }); + it("bot filter: an asset authored by a [bot] login is skipped without fetching", async () => { + const { env } = baseEnv(); + const ref: IngestSourceRef = { repo: REPO, kind: "issues", num: 3, source: "comment:9" }; + const fetchImpl = vi.fn(fakeFetch({ [ASSET_ID]: pngRoute(PNG) })); + const { putImpl, calls } = spyPut(); + + const summary = await reconcileIngestSource( + env, + ws, + WS, + ref, + `see ${ASSET_URL}`, + "claude[bot]", + { fetchImpl, putImpl }, + ); + + expect(fetchImpl).not.toHaveBeenCalled(); + expect(calls).toHaveLength(0); + expect(summary.skipped).toEqual([{ url: ASSET_URL, reason: "bot_author" }]); + expect(await ledgerRow(env.DB, REPO, ASSET_ID)).toBeNull(); + }); + + it("bot filter: ingestBotAuthors deliberately re-admits bot-authored assets", async () => { + const { env } = baseEnv(); + const ref: IngestSourceRef = { repo: REPO, kind: "issues", num: 3, source: "comment:9" }; + const fetchImpl = fakeFetch({ [ASSET_ID]: pngRoute(PNG) }); + const { putImpl, calls } = spyPut(); + + const summary = await reconcileIngestSource( + env, + ws, + WS, + ref, + `see ${ASSET_URL}`, + "claude[bot]", + { fetchImpl, putImpl, ingestBotAuthors: true }, + ); + + expect(calls).toHaveLength(1); + expect(summary.skipped).toEqual([]); + expect(summary.ingested).toHaveLength(1); + }); + + it("small-image filter: an image under the minimum dimension is a permanent too_small skip", async () => { + const { env } = baseEnv(); + const ref: IngestSourceRef = { repo: REPO, kind: "pull", num: 7, source: "body" }; + // Real GIF89a header with a 128×128 logical screen — the emoji/badge + // junk this gate exists for. + const tinyGif = gifOf(128, 128); + const fetchImpl = fakeFetch({ + [ASSET_ID]: () => + new Response(tinyGif, { status: 200, headers: { "content-type": "image/gif" } }), + }); + const { putImpl, calls } = spyPut(); + + const summary = await reconcileIngestSource(env, ws, WS, ref, `see ${ASSET_URL}`, "octocat", { + fetchImpl, + putImpl, + }); + + expect(calls).toHaveLength(0); + expect(summary.skipped).toEqual([{ url: ASSET_URL, reason: "too_small" }]); + expect(await ledgerRow(env.DB, REPO, ASSET_ID)).toBeNull(); + }); + + it("small-image filter: an image at or above the minimum dimension ingests", async () => { + const { env } = baseEnv(); + const ref: IngestSourceRef = { repo: REPO, kind: "pull", num: 7, source: "body" }; + const bigGif = gifOf(800, 600); + const fetchImpl = fakeFetch({ + [ASSET_ID]: () => + new Response(bigGif, { status: 200, headers: { "content-type": "image/gif" } }), + }); + const { putImpl, calls } = spyPut(); + + const summary = await reconcileIngestSource(env, ws, WS, ref, `see ${ASSET_URL}`, "octocat", { + fetchImpl, + putImpl, + }); + + expect(calls).toHaveLength(1); + expect(summary.skipped).toEqual([]); + }); + it("guard skip: asset 404 is a permanent skip", async () => { const { env } = baseEnv(); const ref: IngestSourceRef = { repo: REPO, kind: "pull", num: 7, source: "body" }; @@ -560,9 +645,13 @@ describe("ingestForWebhook", () => { expect(fetchImpl).not.toHaveBeenCalled(); }); - it("link + workspace but knob false: resolves without fetching the issue body or asset", async () => { + it("link + workspace but knob explicitly false: resolves without fetching the issue body or asset", async () => { const { env: base } = baseEnv(); - const env = withRegistry(base, { provider: "r2", bucket: "b" } as WorkspaceRecord); + const env = withRegistry(base, { + provider: "r2", + bucket: "b", + githubIngestAttachments: false, + } as WorkspaceRecord); await recordRepoLink(env.DB, REPO, WS, "test"); const calls: string[] = []; const impl = ((url: string, init: RequestInit = {}) => { @@ -576,13 +665,9 @@ describe("ingestForWebhook", () => { expect(calls.some((u) => u.includes("/issues/7") || u.includes(ASSET_ID))).toBe(false); }); - it("knob true: fetches the issue body, puts, ledgers", async () => { + it("knob unset: ingestion is on by default — fetches the issue body, puts, ledgers", async () => { const { env: base } = baseEnv(); - const env = withRegistry(base, { - provider: "r2", - bucket: "b", - githubIngestAttachments: true, - } as WorkspaceRecord); + const env = withRegistry(base, { provider: "r2", bucket: "b" } as WorkspaceRecord); await recordRepoLink(env.DB, REPO, WS, "test"); const impl = fakeFetch({ "/contents/": () => new Response("nf", { status: 404 }), @@ -603,6 +688,47 @@ describe("ingestForWebhook", () => { expect(row).not.toBeNull(); }); + it("passes the resolved ingestBotAttachments knob through: a repo config enabling it re-admits bot authors", async () => { + const { env: base } = baseEnv(); + const env = withRegistry(base, { provider: "r2", bucket: "b" } as WorkspaceRecord); + await recordRepoLink(env.DB, REPO, WS, "test"); + const impl = fakeFetch({ + "/contents/": () => new Response("comment:\n ingestBotAttachments: true\n", { status: 200 }), + "/repos/acme/app/issues/7": () => + new Response(JSON.stringify({ body: `see ${ASSET_URL}`, user: { login: "claude[bot]" } }), { + status: 200, + }), + [ASSET_ID]: pngRoute(PNG), + }); + const { putImpl, calls } = spyPut(); + const ref: IngestSourceRef = { repo: REPO, kind: "pull", num: 7, source: "body" }; + + await withGlobalFetch(impl, () => ingestForWebhook(env, ref, { fetchImpl: impl, putImpl })); + + expect(calls).toHaveLength(1); + }); + + it("filters a bot-authored body by default (no repo config)", async () => { + const { env: base } = baseEnv(); + const env = withRegistry(base, { provider: "r2", bucket: "b" } as WorkspaceRecord); + await recordRepoLink(env.DB, REPO, WS, "test"); + const impl = fakeFetch({ + "/contents/": () => new Response("nf", { status: 404 }), + "/repos/acme/app/issues/7": () => + new Response(JSON.stringify({ body: `see ${ASSET_URL}`, user: { login: "claude[bot]" } }), { + status: 200, + }), + [ASSET_ID]: pngRoute(PNG), + }); + const { putImpl, calls } = spyPut(); + const ref: IngestSourceRef = { repo: REPO, kind: "pull", num: 7, source: "body" }; + + await withGlobalFetch(impl, () => ingestForWebhook(env, ref, { fetchImpl: impl, putImpl })); + + expect(calls).toHaveLength(0); + expect(await ledgerRow(env.DB, REPO, ASSET_ID)).toBeNull(); + }); + it("comment source 404: reconciles with text null (detach-all), not a throw", async () => { const { env: base } = baseEnv(); const env = withRegistry(base, { diff --git a/apps/api/src/github-ingest.ts b/apps/api/src/github-ingest.ts index 061c34e5..3f4b8207 100644 --- a/apps/api/src/github-ingest.ts +++ b/apps/api/src/github-ingest.ts @@ -43,7 +43,12 @@ import { setLedgerSource, } from "./github-ingest-ledger"; import { updateFileMetadataValue } from "./file-metadata"; -import { detectContentType, maxBytesForContentType, resolveUploadPolicy } from "./guards"; +import { + detectContentType, + detectImageDimensions, + maxBytesForContentType, + resolveUploadPolicy, +} from "./guards"; import { putObject } from "./files-core"; import { findRepoLinkStrict } from "./github-repo-links"; import { resolveRepoCommentOptions } from "./repo-comment-config"; @@ -88,8 +93,24 @@ export interface IngestDeps { * today's behavior. */ mode?: GhKeyMode; + /** + * Re-admit attachments authored by `*[bot]` logins (issue #690 junk + * filter). Absent/false — the default — skips them permanently + * (`bot_author`); webhook ingestion threads the resolved + * `ingestBotAttachments` knob through here. + */ + ingestBotAuthors?: boolean; } +/** + * Floor on either pixel dimension for ingested images (issue #690): the + * emoji, badge, and tracking-pixel junk bots embed is far below it, real + * screenshots are far above. Applies only when the header's dimensions are + * decodable — an undecodable header fails open, since this is an index tier + * whose originals stay on GitHub either way. + */ +const MIN_INGEST_IMAGE_DIMENSION = 200; + function emptySummary(): IngestSummary { return { ingested: [], reattached: [], detached: [], skipped: [] }; } @@ -235,6 +256,14 @@ async function fetchAndStore( return { kind: "skip", reason: "too_large" }; } + const dims = detectImageDimensions(bytes, sniffed); + if ( + dims && + (dims.width < MIN_INGEST_IMAGE_DIMENSION || dims.height < MIN_INGEST_IMAGE_DIMENSION) + ) { + return { kind: "skip", reason: "too_small" }; + } + const mode: GhKeyMode = deps.mode ?? { mode: "plain" }; const key = ingestKeyForMode(mode, ref, attachment.id, extensionForContentType(sniffed)); try { @@ -303,6 +332,12 @@ export async function reconcileIngestSource( await Promise.all(found.map(async (a) => [a.id, await ledgerRow(db, ref.repo, a.id)] as const)), ); + // Bot-author gate applies only to NEW assets: an already-ledgered asset was + // admitted under whatever policy held when it was fetched, so reattach/ + // source-move bookkeeping below still runs for it — junk never enters the + // ledger in the first place, and this loop never re-fetches ledgered rows. + const skipBotAuthor = author !== null && author.endsWith("[bot]") && !deps.ingestBotAuthors; + for (const attachment of found) { const row = rows.get(attachment.id) ?? null; if (row) { @@ -330,6 +365,11 @@ export async function reconcileIngestSource( continue; } + if (skipBotAuthor) { + summary.skipped.push({ url: attachment.url, reason: "bot_author" }); + continue; + } + const result = await fetchAndStore(env, ws, workspaceName, ref, attachment, author, deps); if (result.kind === "skip") { summary.skipped.push({ url: attachment.url, reason: result.reason }); @@ -416,6 +456,7 @@ export async function ingestForWebhook( ...deps, token, mode, + ingestBotAuthors: options.ingestBotAttachments, }); } diff --git a/apps/api/src/guards.ts b/apps/api/src/guards.ts index cee1c3f3..c095048c 100644 --- a/apps/api/src/guards.ts +++ b/apps/api/src/guards.ts @@ -115,6 +115,82 @@ export function detectContentType(bytes: Uint8Array): string | null { return null; } +export interface ImageDimensions { + width: number; + height: number; +} + +/** + * Best-effort pixel dimensions read straight from an image's header — + * PNG (IHDR), GIF (logical screen descriptor), JPEG (first SOF marker), + * WebP (VP8X/VP8/VP8L chunk). Returns `undefined` for any other content + * type or a header it can't decode; callers that gate on dimensions must + * fail open on `undefined` rather than treating it as zero. + */ +export function detectImageDimensions( + bytes: Uint8Array, + contentType: string, +): ImageDimensions | undefined { + const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + const valid = (width: number, height: number) => + width > 0 && height > 0 ? { width, height } : undefined; + + if (contentType === "image/png") { + // 8-byte signature, 4-byte chunk length, "IHDR", then BE uint32 w/h. + if (bytes.length < 24 || !matches(bytes, [0x49, 0x48, 0x44, 0x52], 12)) return undefined; + return valid(view.getUint32(16, false), view.getUint32(20, false)); + } + if (contentType === "image/gif") { + // "GIF8xa" then LE uint16 logical screen width/height. + if (bytes.length < 10) return undefined; + return valid(view.getUint16(6, true), view.getUint16(8, true)); + } + if (contentType === "image/jpeg") { + // Walk marker segments to the first start-of-frame (SOF0–SOF15, minus + // the non-frame DHT/DAC/RST markers in that range). + let i = 2; + while (i + 9 < bytes.length) { + if (bytes[i] !== 0xff) return undefined; + const marker = bytes[i + 1]!; + if (marker === 0xd8 || (marker >= 0xd0 && marker <= 0xd9)) { + i += 2; + continue; + } + const isSof = + marker >= 0xc0 && marker <= 0xcf && marker !== 0xc4 && marker !== 0xc8 && marker !== 0xcc; + if (isSof) { + return valid(view.getUint16(i + 7, false), view.getUint16(i + 5, false)); + } + i += 2 + view.getUint16(i + 2, false); + } + return undefined; + } + if (contentType === "image/webp") { + if (bytes.length < 30) return undefined; + const chunk = asciiAt(bytes, 12, 4); + if (chunk === "VP8X") { + // 24-bit LE canvas width-1 / height-1 at payload offsets 4 and 7. + const w = bytes[24]! | (bytes[25]! << 8) | (bytes[26]! << 16); + const h = bytes[27]! | (bytes[28]! << 8) | (bytes[29]! << 16); + return valid(w + 1, h + 1); + } + if (chunk === "VP8 ") { + // Lossy bitstream: 14-bit LE dimensions after the 3-byte frame tag + + // 3-byte start code (9D 01 2A). + if (!matches(bytes, [0x9d, 0x01, 0x2a], 23)) return undefined; + return valid(view.getUint16(26, true) & 0x3fff, view.getUint16(28, true) & 0x3fff); + } + if (chunk === "VP8L") { + // Lossless bitstream: signature 0x2f then 14-bit width-1/height-1. + if (bytes[20] !== 0x2f) return undefined; + const bits = view.getUint32(21, true); + return valid((bits & 0x3fff) + 1, ((bits >> 14) & 0x3fff) + 1); + } + return undefined; + } + return undefined; +} + export type UploadRejection = { ok: false; status: 413 | 415; diff --git a/apps/api/src/routes/me.test.ts b/apps/api/src/routes/me.test.ts index 7983871c..5e18321a 100644 --- a/apps/api/src/routes/me.test.ts +++ b/apps/api/src/routes/me.test.ts @@ -2313,7 +2313,8 @@ describe("GET /me/workspaces/:name/comment-preview", () => { metaState: true, linkToFilePage: true, note: null, - ingestGithubAttachments: false, + ingestGithubAttachments: true, + ingestBotAttachments: false, }); expect(body.source).toMatchObject({ imageWidth: "auto" }); expect(typeof body.body).toBe("string"); diff --git a/apps/api/test/guards.test.ts b/apps/api/test/guards.test.ts index 092aa458..6625d5db 100644 --- a/apps/api/test/guards.test.ts +++ b/apps/api/test/guards.test.ts @@ -4,9 +4,11 @@ import { DEFAULT_ALLOWED_CONTENT_TYPES, DEFAULT_MAX_UPLOAD_BYTES, detectContentType, + detectImageDimensions, inspectUpload, resolveUploadPolicy, } from "../src/guards"; +import { gifOf, pngOf } from "./helpers/image-fixtures"; const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0]); const JPEG = new Uint8Array([0xff, 0xd8, 0xff, 0xe0, 0, 0]); @@ -48,6 +50,51 @@ describe("detectContentType", () => { }); }); +describe("detectImageDimensions", () => { + it("reads PNG dimensions from the IHDR chunk", () => { + expect(detectImageDimensions(pngOf(800, 600), "image/png")).toEqual({ + width: 800, + height: 600, + }); + }); + + it("reads GIF dimensions from the logical screen descriptor", () => { + expect(detectImageDimensions(gifOf(128, 128), "image/gif")).toEqual({ + width: 128, + height: 128, + }); + }); + + it("reads JPEG dimensions from the first SOF marker", () => { + // FFD8, APP0 stub, then SOF0 with height 480 / width 640. + const bytes = new Uint8Array([ + 0xff, 0xd8, 0xff, 0xe0, 0x00, 0x04, 0x00, 0x00, 0xff, 0xc0, 0x00, 0x11, 0x08, 0x01, 0xe0, + 0x02, 0x80, 0x03, + ]); + expect(detectImageDimensions(bytes, "image/jpeg")).toEqual({ width: 640, height: 480 }); + }); + + it("reads WebP dimensions from a VP8X chunk", () => { + const bytes = new Uint8Array(30); + bytes.set([0x52, 0x49, 0x46, 0x46]); // RIFF + bytes.set([0x57, 0x45, 0x42, 0x50], 8); // WEBP + bytes.set([0x56, 0x50, 0x38, 0x58], 12); // VP8X + // canvas width-1 = 639, height-1 = 479 as 24-bit LE at offsets 24/27 + bytes[24] = 0x7f; + bytes[25] = 0x02; + bytes[27] = 0xdf; + bytes[28] = 0x01; + expect(detectImageDimensions(bytes, "image/webp")).toEqual({ width: 640, height: 480 }); + }); + + it("returns undefined for truncated headers and non-image types", () => { + expect(detectImageDimensions(new Uint8Array([0x89, 0x50]), "image/png")).toBeUndefined(); + expect(detectImageDimensions(gifOf(128, 128).subarray(0, 7), "image/gif")).toBeUndefined(); + expect(detectImageDimensions(new Uint8Array(30), "video/webm")).toBeUndefined(); + expect(detectImageDimensions(new Uint8Array(0), "image/jpeg")).toBeUndefined(); + }); +}); + describe("checkDeclaredLength", () => { const policy = resolveUploadPolicy({ maxUploadBytes: 100 }); diff --git a/apps/api/test/helpers/image-fixtures.ts b/apps/api/test/helpers/image-fixtures.ts new file mode 100644 index 00000000..2d34aca8 --- /dev/null +++ b/apps/api/test/helpers/image-fixtures.ts @@ -0,0 +1,24 @@ +/** + * Minimal image headers with real, decodable dimensions — just enough bytes + * for `detectContentType`/`detectImageDimensions` (guards.ts) to sniff, no + * actual pixel data. + */ + +/** PNG signature + IHDR chunk with the given dimensions. */ +export function pngOf(width: number, height: number): Uint8Array { + const bytes = new Uint8Array(24); + bytes.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 13]); + bytes.set([0x49, 0x48, 0x44, 0x52], 12); // "IHDR" + new DataView(bytes.buffer).setUint32(16, width, false); + new DataView(bytes.buffer).setUint32(20, height, false); + return bytes; +} + +/** GIF89a header + logical screen descriptor with the given dimensions. */ +export function gifOf(width: number, height: number): Uint8Array { + const bytes = new Uint8Array(13); + bytes.set([0x47, 0x49, 0x46, 0x38, 0x39, 0x61]); // "GIF89a" + new DataView(bytes.buffer).setUint16(6, width, true); + new DataView(bytes.buffer).setUint16(8, height, true); + return bytes; +} diff --git a/apps/web/src/pages/account/workspaces/[name]/settings.astro b/apps/web/src/pages/account/workspaces/[name]/settings.astro index 7cfc039a..05a63528 100644 --- a/apps/web/src/pages/account/workspaces/[name]/settings.astro +++ b/apps/web/src/pages/account/workspaces/[name]/settings.astro @@ -176,7 +176,7 @@ const tip = {

Mirror manually added images and video on pull requests and issues into this - workspace. + workspace. On by default; bot-authored and tiny images are filtered out.

@@ -931,7 +931,9 @@ const tip = { // default on, import defaults off. paintToggle(metaToggle, settings.showMetadata ?? true); paintToggle(linkToggle, settings.linkToFilePage ?? true); - paintToggle(ingestToggle, settings.ingestGithubAttachments ?? false); + // Unset means the platform default, which is ON (issue #690) — paint + // the toggle the way ingestion will actually behave. + paintToggle(ingestToggle, settings.ingestGithubAttachments ?? true); noteInput.value = settings.note ?? ""; noteCount.textContent = String(noteInput.value.length); // Baseline off the painted form, not `settings`: the toggles normalise diff --git a/apps/web/src/pages/docs/comment-config.astro b/apps/web/src/pages/docs/comment-config.astro index 72ad70bf..58889f4d 100644 --- a/apps/web/src/pages/docs/comment-config.astro +++ b/apps/web/src/pages/docs/comment-config.astro @@ -33,8 +33,9 @@ const TOC = [ >

- Every key is optional and lives under a top-level comment: map. All of it is presentation - only — it changes how the managed comment looks, never what gets uploaded or where. + Every key is optional and lives under a top-level comment: map. Most of it is presentation + — how the managed comment looks — plus two keys that control whether GitHub-posted attachments get + imported into the workspace.

- Nothing in the file can change which files get uploaded, where they're hosted, or which repo a - workspace can post to — it only reshapes the comment body for a repo that's already allowed to - receive one. + Nothing in the file can change where files are hosted or which repo a workspace can post to — + it reshapes the comment body for a repo that's already allowed to receive one, and gates + whether that repo's GitHub-posted attachments get mirrored in.

@@ -75,6 +80,15 @@ const TOC = [ # setting. linkToFilePage: true + # Whether images and video dropped directly on GitHub PRs and issues get + # imported into the workspace. On by default. + ingestGithubAttachments: true + + # Whether attachments authored by [bot] accounts are imported too. + # Off by default — bot-posted media is usually noise (status GIFs, badges). + # Tiny images (under 200px on either side) are always skipped. + ingestBotAttachments: false + # Optional short markdown at the top of the comment, before the media — # repo-specific context, a link to a contributing guide, and so on. # Trimmed, capped at 500 characters, rendered as-is: it's diff --git a/apps/web/src/pages/docs/github-app.astro b/apps/web/src/pages/docs/github-app.astro index d465af2e..9bf43f89 100644 --- a/apps/web/src/pages/docs/github-app.astro +++ b/apps/web/src/pages/docs/github-app.astro @@ -9,6 +9,7 @@ const TOC = [ { id: "what-it-adds", label: "What it adds" }, { id: "install", label: "Install it" }, { id: "staging", label: "Staging before a PR exists" }, + { id: "ingest", label: "Imported attachments" }, { id: "permissions", label: "Permissions & events" }, { id: "bindings", label: "Repo bindings" }, { id: "self-healing", label: "Self-healing comments" }, @@ -49,7 +50,7 @@ const TOC = [

Installing the App is the recommended setup — it's what makes the staged - loop hands-off. Four things change: + loop hands-off. Five things change:

Everything in these docs still works without it — see @@ -135,6 +141,39 @@ const TOC = [

+
+

+ Imported attachments # +

+

+ When someone drags an image or video into a PR or issue on GitHub itself, the App imports it + into the workspace automatically. Imported files carry the same gh.* metadata as CLI + uploads, so they show up in search and on the "From GitHub" rail of file pages — and they survive + even if GitHub's own attachment copy goes away. They never appear in the managed attachments comment; + they're an index, not a source of truth. +

+

+ Importing is on by default for linked repos. Turn it off per workspace on the workspace's + comment settings page, or per repo with + ingestGithubAttachments: false in .uploads.yml. +

+

Two filters keep low-value files out:

+ +
+

Permissions & events { expect(parseRepoCommentConfig("- a\n- b\n", "yaml").config).toBeNull(); expect(parseRepoCommentConfig("other: 1\n", "yaml").config).toBeNull(); }); + it("parses ingestBotAttachments as a boolean, warns on other types", () => { + expect( + parseRepoCommentConfig("comment:\n ingestBotAttachments: true\n", "yaml").config, + ).toEqual({ ingestBotAttachments: true }); + const { config, warnings } = parseRepoCommentConfig( + 'comment:\n ingestBotAttachments: "yes"\n', + "yaml", + ); + expect(config).toEqual({}); + expect(warnings[0]).toContain("ingestBotAttachments"); + }); it("never throws on hostile input", () => { for (const text of ["", "\0", "!!js/function 'x'", "{", "comment: 3"]) { expect(() => parseRepoCommentConfig(text, "yaml")).not.toThrow(); @@ -152,6 +163,22 @@ describe("resolveCommentOptions", () => { expect(options.metaPath).toBe(true); expect(options.metaState).toBe(false); }); + it("defaults ingestGithubAttachments on and ingestBotAttachments off", () => { + const { options } = resolveCommentOptions(null, null); + expect(options.ingestGithubAttachments).toBe(true); + expect(options.ingestBotAttachments).toBe(false); + }); + it("resolves ingestBotAttachments with repo > workspace precedence", () => { + const { options, source } = resolveCommentOptions( + { ingestBotAttachments: true }, + { ingestBotAttachments: false }, + ); + expect(options.ingestBotAttachments).toBe(true); + expect(source.ingestBotAttachments).toBe("repo"); + const ws = resolveCommentOptions(null, { ingestBotAttachments: true }); + expect(ws.options.ingestBotAttachments).toBe(true); + expect(ws.source.ingestBotAttachments).toBe("workspace"); + }); }); describe("comment-config-golden.json parity (canonical side)", () => { diff --git a/packages/comment-config/src/index.ts b/packages/comment-config/src/index.ts index d049e6a8..5341fbe0 100644 --- a/packages/comment-config/src/index.ts +++ b/packages/comment-config/src/index.ts @@ -17,6 +17,7 @@ export interface RepoCommentConfig { linkToFilePage?: boolean; note?: string; ingestGithubAttachments?: boolean; + ingestBotAttachments?: boolean; } export interface WorkspaceCommentDefaults { imageWidth?: "full" | number; @@ -25,6 +26,7 @@ export interface WorkspaceCommentDefaults { linkToFilePage?: boolean; note?: string; ingestGithubAttachments?: boolean; + ingestBotAttachments?: boolean; } export interface ResolvedCommentOptions { imageWidth: "auto" | "full" | number; @@ -34,6 +36,7 @@ export interface ResolvedCommentOptions { linkToFilePage: boolean; note: string | null; ingestGithubAttachments: boolean; + ingestBotAttachments: boolean; } export type OptionSource = "repo" | "workspace" | "auto"; @@ -44,7 +47,8 @@ export const AUTO_COMMENT_OPTIONS: ResolvedCommentOptions = { metaState: true, linkToFilePage: true, note: null, - ingestGithubAttachments: false, + ingestGithubAttachments: true, + ingestBotAttachments: false, }; export const NOTE_MAX_CHARS = 500; @@ -107,6 +111,13 @@ export function parseRepoCommentConfig( else warnings.push(`ingestGithubAttachments: expected a boolean; dropped`); } + // ingestBotAttachments: boolean + if ("ingestBotAttachments" in c) { + const v = c.ingestBotAttachments; + if (typeof v === "boolean") config.ingestBotAttachments = v; + else warnings.push(`ingestBotAttachments: expected a boolean; dropped`); + } + // meta.path / meta.state: booleans nested under `meta` if ("meta" in c) { const v = c.meta; @@ -160,6 +171,9 @@ export function resolveCommentOptions( ...(ws?.ingestGithubAttachments !== undefined ? { ingestGithubAttachments: ws.ingestGithubAttachments } : {}), + ...(ws?.ingestBotAttachments !== undefined + ? { ingestBotAttachments: ws.ingestBotAttachments } + : {}), }; const options = { ...AUTO_COMMENT_OPTIONS }; const source = Object.fromEntries( @@ -173,6 +187,7 @@ export function resolveCommentOptions( "metaState", "linkToFilePage", "ingestGithubAttachments", + "ingestBotAttachments", ] as const) { if (cfg[key] !== undefined && source[key] === "auto") { (options as Record)[key] = cfg[key]; diff --git a/packages/uploads/src/commands.ts b/packages/uploads/src/commands.ts index 60db2e7a..c6a61da2 100644 --- a/packages/uploads/src/commands.ts +++ b/packages/uploads/src/commands.ts @@ -3385,7 +3385,9 @@ Scans the PR/issue description and comments for github.com/user-attachments media, mirrors new ones into the workspace (indexed, not added to the managed comment), and detaches ones no longer referenced. Works on any repo linked to the workspace; the .uploads.yml ingestGithubAttachments knob only gates the -automatic webhook path. +automatic webhook path. Bot-authored attachments and images under 200px on +either side are always skipped (the .uploads.yml ingestBotAttachments knob +re-admits bot media on the webhook path only). Examples: uploads ingest --pr 123 diff --git a/packages/uploads/src/comment-config.generated.ts b/packages/uploads/src/comment-config.generated.ts index 500c8a16..9b575ae8 100644 --- a/packages/uploads/src/comment-config.generated.ts +++ b/packages/uploads/src/comment-config.generated.ts @@ -15,6 +15,7 @@ export interface RepoCommentConfig { linkToFilePage?: boolean; note?: string; ingestGithubAttachments?: boolean; + ingestBotAttachments?: boolean; } export interface WorkspaceCommentDefaults { imageWidth?: "full" | number; @@ -23,6 +24,7 @@ export interface WorkspaceCommentDefaults { linkToFilePage?: boolean; note?: string; ingestGithubAttachments?: boolean; + ingestBotAttachments?: boolean; } export interface ResolvedCommentOptions { imageWidth: "auto" | "full" | number; @@ -32,6 +34,7 @@ export interface ResolvedCommentOptions { linkToFilePage: boolean; note: string | null; ingestGithubAttachments: boolean; + ingestBotAttachments: boolean; } export type OptionSource = "repo" | "workspace" | "auto"; @@ -42,7 +45,8 @@ export const AUTO_COMMENT_OPTIONS: ResolvedCommentOptions = { metaState: true, linkToFilePage: true, note: null, - ingestGithubAttachments: false, + ingestGithubAttachments: true, + ingestBotAttachments: false, }; export const NOTE_MAX_CHARS = 500; @@ -105,6 +109,13 @@ export function parseRepoCommentConfig( else warnings.push(`ingestGithubAttachments: expected a boolean; dropped`); } + // ingestBotAttachments: boolean + if ("ingestBotAttachments" in c) { + const v = c.ingestBotAttachments; + if (typeof v === "boolean") config.ingestBotAttachments = v; + else warnings.push(`ingestBotAttachments: expected a boolean; dropped`); + } + // meta.path / meta.state: booleans nested under `meta` if ("meta" in c) { const v = c.meta; @@ -158,6 +169,9 @@ export function resolveCommentOptions( ...(ws?.ingestGithubAttachments !== undefined ? { ingestGithubAttachments: ws.ingestGithubAttachments } : {}), + ...(ws?.ingestBotAttachments !== undefined + ? { ingestBotAttachments: ws.ingestBotAttachments } + : {}), }; const options = { ...AUTO_COMMENT_OPTIONS }; const source = Object.fromEntries( @@ -171,6 +185,7 @@ export function resolveCommentOptions( "metaState", "linkToFilePage", "ingestGithubAttachments", + "ingestBotAttachments", ] as const) { if (cfg[key] !== undefined && source[key] === "auto") { (options as Record)[key] = cfg[key]; diff --git a/test/fixtures/comment-config-golden.json b/test/fixtures/comment-config-golden.json index ddc40815..8df456b5 100644 --- a/test/fixtures/comment-config-golden.json +++ b/test/fixtures/comment-config-golden.json @@ -188,7 +188,9 @@ "text": "comment:\n ingestGithubAttachments: true\n", "format": "yaml", "expected": { - "config": { "ingestGithubAttachments": true }, + "config": { + "ingestGithubAttachments": true + }, "warnings": [] } }, @@ -200,6 +202,17 @@ "config": {}, "warnings": ["ingestGithubAttachments: expected a boolean; dropped"] } + }, + { + "name": "ingestBotAttachments boolean parses; non-boolean dropped with warning", + "text": "comment:\n ingestBotAttachments: true\n", + "format": "yaml", + "expected": { + "config": { + "ingestBotAttachments": true + }, + "warnings": [] + } } ], "resolveCases": [ @@ -215,7 +228,8 @@ "metaState": true, "linkToFilePage": true, "note": null, - "ingestGithubAttachments": false + "ingestGithubAttachments": true, + "ingestBotAttachments": false }, "source": { "imageWidth": "auto", @@ -224,7 +238,8 @@ "metaState": "auto", "linkToFilePage": "auto", "note": "auto", - "ingestGithubAttachments": "auto" + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "auto" } } }, @@ -245,7 +260,8 @@ "metaState": true, "linkToFilePage": true, "note": null, - "ingestGithubAttachments": false + "ingestGithubAttachments": true, + "ingestBotAttachments": false }, "source": { "imageWidth": "repo", @@ -254,7 +270,8 @@ "metaState": "auto", "linkToFilePage": "auto", "note": "auto", - "ingestGithubAttachments": "auto" + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "auto" } } }, @@ -272,7 +289,8 @@ "metaState": false, "linkToFilePage": true, "note": null, - "ingestGithubAttachments": false + "ingestGithubAttachments": true, + "ingestBotAttachments": false }, "source": { "imageWidth": "auto", @@ -281,7 +299,8 @@ "metaState": "workspace", "linkToFilePage": "auto", "note": "auto", - "ingestGithubAttachments": "auto" + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "auto" } } }, @@ -301,7 +320,8 @@ "metaState": false, "linkToFilePage": true, "note": null, - "ingestGithubAttachments": false + "ingestGithubAttachments": true, + "ingestBotAttachments": false }, "source": { "imageWidth": "auto", @@ -310,14 +330,19 @@ "metaState": "workspace", "linkToFilePage": "auto", "note": "auto", - "ingestGithubAttachments": "auto" + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "auto" } } }, { "name": "ingest knob repo over workspace", - "repo": { "ingestGithubAttachments": true }, - "workspace": { "ingestGithubAttachments": false }, + "repo": { + "ingestGithubAttachments": true + }, + "workspace": { + "ingestGithubAttachments": false + }, "expected": { "options": { "imageWidth": "auto", @@ -326,7 +351,39 @@ "metaState": true, "linkToFilePage": true, "note": null, - "ingestGithubAttachments": true + "ingestGithubAttachments": true, + "ingestBotAttachments": false + }, + "source": { + "imageWidth": "auto", + "maxInlineImages": "auto", + "metaPath": "auto", + "metaState": "auto", + "linkToFilePage": "auto", + "note": "auto", + "ingestGithubAttachments": "repo", + "ingestBotAttachments": "auto" + } + } + }, + { + "name": "bot-ingest knob repo over workspace; ingest default is on", + "repo": { + "ingestBotAttachments": true + }, + "workspace": { + "ingestBotAttachments": false + }, + "expected": { + "options": { + "imageWidth": "auto", + "maxInlineImages": 16, + "metaPath": true, + "metaState": true, + "linkToFilePage": true, + "note": null, + "ingestGithubAttachments": true, + "ingestBotAttachments": true }, "source": { "imageWidth": "auto", @@ -335,7 +392,8 @@ "metaState": "auto", "linkToFilePage": "auto", "note": "auto", - "ingestGithubAttachments": "repo" + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "repo" } } }