From b32b328b87020deedc7525d372c30823993595c7 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 11 May 2026 15:42:44 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Harden=20Artifac?= =?UTF-8?q?t=20Routing=20Lookups=20(#13069)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/utils/__tests__/artifacts.test.ts | 31 ++++++++++++++ client/src/utils/artifacts.ts | 45 ++++++++++++++------ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/client/src/utils/__tests__/artifacts.test.ts b/client/src/utils/__tests__/artifacts.test.ts index 63c6fbf4da..be65a095e1 100644 --- a/client/src/utils/__tests__/artifacts.test.ts +++ b/client/src/utils/__tests__/artifacts.test.ts @@ -426,6 +426,29 @@ describe('detectArtifactTypeFromFile', () => { expect(detectArtifactTypeFromFile({ filename: 'LICENSE', type: '', text: 'MIT' })).toBeNull(); }); + it.each(['constructor', '__proto__'])( + 'does not classify inherited Object property "%s" from filename or MIME lookups', + (key) => { + expect( + detectArtifactTypeFromFile({ filename: key, type: '', text: '' }), + ).toBeNull(); + expect( + detectArtifactTypeFromFile({ + filename: `payload.${key}`, + type: '', + text: '', + }), + ).toBeNull(); + expect( + detectArtifactTypeFromFile({ + filename: 'payload.bin', + type: key, + text: '', + }), + ).toBeNull(); + }, + ); + /* Codex review P3 companion: `extensionOf` used to consider the * whole path string, so `pkg.v1/Dockerfile` yielded a path-laden * "extension" that masked the bare-name fallback. The basename- @@ -831,6 +854,14 @@ describe('fileToArtifact', () => { expect(artifact!.type).toBe(TOOL_ARTIFACT_TYPES.PLAIN_TEXT); }); + it('rejects a runtime bogus preClassifiedType before constructing an artifact', () => { + const artifact = fileToArtifact( + { ...baseFile, filename: 'constructor', type: '', text: '' }, + { preClassifiedType: Object as unknown as ToolArtifactType }, + ); + expect(artifact).toBeNull(); + }); + it.each([[TOOL_ARTIFACT_TYPES.HTML], [TOOL_ARTIFACT_TYPES.REACT], [TOOL_ARTIFACT_TYPES.MERMAID]])( 'returns null when preClassifiedType=%s is paired with empty text (defense in depth)', (preClassifiedType) => { diff --git a/client/src/utils/artifacts.ts b/client/src/utils/artifacts.ts index 8244adf101..264c8f9e30 100644 --- a/client/src/utils/artifacts.ts +++ b/client/src/utils/artifacts.ts @@ -296,6 +296,15 @@ export const TOOL_ARTIFACT_TYPES = { export type ToolArtifactType = (typeof TOOL_ARTIFACT_TYPES)[keyof typeof TOOL_ARTIFACT_TYPES]; +const TOOL_ARTIFACT_TYPE_VALUES: ReadonlySet = new Set(Object.values(TOOL_ARTIFACT_TYPES)); + +export function isToolArtifactType(type: unknown): type is ToolArtifactType { + return typeof type === 'string' && TOOL_ARTIFACT_TYPE_VALUES.has(type); +} + +const lookupOwn = (record: Record, key: string): T | undefined => + Object.prototype.hasOwnProperty.call(record, key) ? record[key] : undefined; + /** * Artifact types whose preview is server-rendered HTML — there's no * source for a "code" view because the underlying file is binary, and @@ -478,19 +487,21 @@ export function languageForFilename( ): string { const ext = extensionOf(filename); if (ext) { - return CODE_EXTENSION_TO_LANGUAGE[ext] ?? ext; + return lookupOwn(CODE_EXTENSION_TO_LANGUAGE, ext) ?? ext; } /* Extensionless filename: try the basename. `Dockerfile` → * `dockerfile` → `'dockerfile'` language hint. */ const bare = bareNameOf(filename); - if (bare && Object.prototype.hasOwnProperty.call(CODE_EXTENSION_TO_LANGUAGE, bare)) { - return CODE_EXTENSION_TO_LANGUAGE[bare]; + const bareLanguage = lookupOwn(CODE_EXTENSION_TO_LANGUAGE, bare); + if (bareLanguage != null) { + return bareLanguage; } /* MIME fallback for the extensionless-name + useful-MIME case. */ if (mime) { const stripped = baseMime(mime); - if (Object.prototype.hasOwnProperty.call(MIME_TO_LANGUAGE, stripped)) { - return MIME_TO_LANGUAGE[stripped]; + const mimeLanguage = lookupOwn(MIME_TO_LANGUAGE, stripped); + if (mimeLanguage != null) { + return mimeLanguage; } } return ''; @@ -585,7 +596,7 @@ const EXTENSION_TO_TOOL_ARTIFACT_TYPE: Record = { * mistake — they ARE source code) shouldn't silently break the React * routing path. The explicit map entries above always win. */ for (const ext of Object.keys(CODE_EXTENSION_TO_LANGUAGE)) { - if (ext in EXTENSION_TO_TOOL_ARTIFACT_TYPE) continue; + if (lookupOwn(EXTENSION_TO_TOOL_ARTIFACT_TYPE, ext) != null) continue; EXTENSION_TO_TOOL_ARTIFACT_TYPE[ext] = TOOL_ARTIFACT_TYPES.CODE; } @@ -691,15 +702,15 @@ export function detectArtifactTypeFromFile( * `bareNameOf(filename)` would otherwise split path separators * twice on the same input. */ const base = attachment.filename ? basenameOf(attachment.filename) : ''; - const byExtension = EXTENSION_TO_TOOL_ARTIFACT_TYPE[extensionFromBasename(base)]; /* Bare-name fallback for extensionless build files (`Dockerfile`, * `Makefile`, `Gemfile`, `Rakefile`, `Vagrantfile`, `Brewfile`). Only * fires when the extension lookup missed AND the basename is in the * routing map; everything else stays on the existing extension/MIME * paths. */ + const byExtension = lookupOwn(EXTENSION_TO_TOOL_ARTIFACT_TYPE, extensionFromBasename(base)); const byBareName = byExtension ? undefined - : EXTENSION_TO_TOOL_ARTIFACT_TYPE[bareNameFromBasename(base)]; + : lookupOwn(EXTENSION_TO_TOOL_ARTIFACT_TYPE, bareNameFromBasename(base)); /* Exact-match MIME lookup first; for the spreadsheet bucket the * backend's `officeHtmlBucket` accepts the broad `excelMimeTypes` * regex (covers `application/x-ms-excel`, `application/x-xls`, @@ -709,10 +720,10 @@ export function detectArtifactTypeFromFile( * routed/registered on the panel. */ const normalizedMime = baseMime(attachment.type); const byMime = - MIME_TO_TOOL_ARTIFACT_TYPE[normalizedMime] ?? + lookupOwn(MIME_TO_TOOL_ARTIFACT_TYPE, normalizedMime) ?? (excelMimeTypes.test(normalizedMime) ? TOOL_ARTIFACT_TYPES.SPREADSHEET : undefined); const type = byExtension ?? byBareName ?? byMime ?? null; - if (type == null) { + if (!isToolArtifactType(type)) { return null; } /* SECURITY GATE: office HTML buckets inject `attachment.text` into @@ -816,12 +827,22 @@ export function fileToArtifact( // fields the function never strictly needs. attachment: Partial< Pick & - Pick + Pick< + TFile, + | 'file_id' + | 'filename' + | 'filepath' + | 'type' + | 'text' + | 'textFormat' + | 'updatedAt' + | 'createdAt' + > >, options?: FileToArtifactOptions, ): Artifact | null { const type = options?.preClassifiedType ?? detectArtifactTypeFromFile(attachment); - if (!type) { + if (!isToolArtifactType(type)) { return null; } // Mirror the empty-text gate from `detectArtifactTypeFromFile` so a