Skip to content

fix(sveltekit): Fix file system race condition in source map cleaning#19714

Merged
chargome merged 1 commit intodevelopfrom
fix/sveltekit-sourcemap-toctou-race
Mar 9, 2026
Merged

fix(sveltekit): Fix file system race condition in source map cleaning#19714
chargome merged 1 commit intodevelopfrom
fix/sveltekit-sourcemap-toctou-race

Conversation

@chargome
Copy link
Member

@chargome chargome commented Mar 9, 2026

Replace existsSync guard with try/catch around read+write operations to eliminate the time-of-check to time-of-use race condition (CWE-367) flagged by CodeQL (code-scanning alert #439).

closes https://github.com/getsentry/sentry-javascript/security/code-scanning/439

…leaning

Replace `existsSync` guard with try/catch around read+write operations
to eliminate the time-of-check to time-of-use race condition (CWE-367)
flagged by CodeQL (code-scanning alert #439).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@chargome chargome self-assigned this Mar 9, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 25.64 kB +0.05% +12 B 🔺
@sentry/browser - with treeshaking flags 24.14 kB +0.03% +7 B 🔺
@sentry/browser (incl. Tracing) 42.44 kB +0.02% +8 B 🔺
@sentry/browser (incl. Tracing, Profiling) 47.1 kB +0.02% +8 B 🔺
@sentry/browser (incl. Tracing, Replay) 81.26 kB +0.02% +9 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 70.88 kB +0.02% +8 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 85.95 kB +0.02% +9 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 98.21 kB +0.01% +7 B 🔺
@sentry/browser (incl. Feedback) 42.44 kB +0.02% +7 B 🔺
@sentry/browser (incl. sendFeedback) 30.31 kB +0.04% +11 B 🔺
@sentry/browser (incl. FeedbackAsync) 35.36 kB +0.04% +11 B 🔺
@sentry/browser (incl. Metrics) 26.8 kB +0.04% +9 B 🔺
@sentry/browser (incl. Logs) 26.95 kB +0.03% +8 B 🔺
@sentry/browser (incl. Metrics & Logs) 27.62 kB +0.04% +9 B 🔺
@sentry/react 27.39 kB +0.04% +9 B 🔺
@sentry/react (incl. Tracing) 44.78 kB +0.02% +8 B 🔺
@sentry/vue 30.09 kB +0.04% +10 B 🔺
@sentry/vue (incl. Tracing) 44.31 kB +0.03% +9 B 🔺
@sentry/svelte 25.66 kB +0.04% +9 B 🔺
CDN Bundle 28.18 kB +0.04% +9 B 🔺
CDN Bundle (incl. Tracing) 43.27 kB +0.03% +11 B 🔺
CDN Bundle (incl. Logs, Metrics) 29.02 kB +0.04% +9 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) 44.11 kB +0.03% +10 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) 68.1 kB +0.02% +8 B 🔺
CDN Bundle (incl. Tracing, Replay) 80.15 kB +0.02% +12 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 81.01 kB +0.02% +10 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 85.66 kB +0.02% +9 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 86.54 kB +0.02% +10 B 🔺
CDN Bundle - uncompressed 82.38 kB +0.04% +26 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 128.09 kB +0.03% +26 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed 85.21 kB +0.04% +26 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 130.93 kB +0.02% +26 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 208.88 kB +0.02% +26 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 244.98 kB +0.02% +26 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 247.8 kB +0.02% +26 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 257.89 kB +0.02% +26 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 260.7 kB +0.01% +26 B 🔺
@sentry/nextjs (client) 47.19 kB +0.02% +9 B 🔺
@sentry/sveltekit (client) 42.9 kB +0.02% +8 B 🔺
@sentry/node-core 52.27 kB +0.07% +35 B 🔺
@sentry/node 174.77 kB +0.04% +62 B 🔺
@sentry/node - without tracing 97.44 kB +0.06% +53 B 🔺
@sentry/aws-serverless 113.24 kB +0.05% +49 B 🔺

View base workflow run

@chargome chargome marked this pull request as ready for review March 9, 2026 13:15
@chargome chargome requested a review from Lms24 March 9, 2026 13:15
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Overly broad catch silently swallows non-ENOENT errors
    • Changed catch block to check for ENOENT errors specifically and log unexpected errors in debug mode, following the existing pattern in the same file.
  • ✅ Fixed: Fix PR missing regression test for the change
    • Added two regression tests that verify ENOENT errors are handled gracefully and unexpected errors are logged when debug is enabled.

Create PR

Or push these changes by commenting:

@cursor push 96a66011b4
Preview (96a66011b4)
diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts
--- a/packages/sveltekit/src/vite/sourceMaps.ts
+++ b/packages/sveltekit/src/vite/sourceMaps.ts
@@ -221,8 +221,12 @@
             '',
           );
           await fs.promises.writeFile(mapFile, cleanedMapContent);
-        } catch {
-          // Map file doesn't exist, nothing to clean
+        } catch (e) {
+          const isKnownError = e instanceof Error && e.message.includes('ENOENT: no such file or directory, open');
+          if (debug && !isKnownError) {
+            // eslint-disable-next-line no-console
+            console.error('[Source Maps Plugin] error while cleaning source map', mapFile, e);
+          }
         }
       }
 

diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts
--- a/packages/sveltekit/test/vite/sourceMaps.test.ts
+++ b/packages/sveltekit/test/vite/sourceMaps.test.ts
@@ -1,4 +1,5 @@
 import { sentryVitePlugin } from '@sentry/vite-plugin';
+import * as fs from 'fs';
 import type { Plugin } from 'vite';
 import * as vite from 'vite';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
@@ -37,6 +38,20 @@
   };
 });
 
+vi.mock('fs', async () => {
+  const actual = (await vi.importActual('fs')) as typeof fs;
+  return {
+    ...actual,
+    existsSync: vi.fn(actual.existsSync),
+    readdirSync: vi.fn(actual.readdirSync),
+    promises: {
+      ...actual.promises,
+      readFile: vi.fn(actual.promises.readFile),
+      writeFile: vi.fn(actual.promises.writeFile),
+    },
+  };
+});
+
 beforeEach(() => {
   vi.clearAllMocks();
 });
@@ -212,6 +227,57 @@
     expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to upload source maps'));
     expect(consoleLogSpy).toHaveBeenCalled();
   });
+
+  it('handles missing map files gracefully during source map cleaning', async () => {
+    const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    vi.mocked(fs.existsSync).mockReturnValueOnce(true);
+    vi.mocked(fs.readdirSync).mockReturnValueOnce([{ name: 'test.js', isDirectory: () => false } as any]);
+    vi.mocked(fs.promises.readFile).mockRejectedValueOnce(
+      Object.assign(new Error('ENOENT: no such file or directory, open'), { code: 'ENOENT' }),
+    );
+
+    const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin');
+
+    // @ts-expect-error this function exists!
+    plugin.configResolved({ build: { ssr: true } });
+    // @ts-expect-error this function exists!
+    await plugin.closeBundle();
+
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
+  });
+
+  it('logs unexpected errors during source map cleaning when debug is enabled', async () => {
+    const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+    vi.mocked(fs.existsSync).mockReturnValueOnce(true);
+    vi.mocked(fs.readdirSync).mockReturnValueOnce([{ name: 'test.js', isDirectory: () => false } as any]);
+    vi.mocked(fs.promises.readFile).mockRejectedValueOnce(new Error('Permission denied'));
+
+    const plugins = await makeCustomSentryVitePlugins(
+      {
+        authToken: 'token',
+        org: 'org',
+        project: 'project',
+        adapter: 'other',
+        debug: true,
+      },
+      { kit: {} },
+    );
+
+    const plugin = plugins.find(p => p.name === 'sentry-sveltekit-debug-id-upload-plugin');
+
+    // @ts-expect-error this function exists!
+    plugin.configResolved({ build: { ssr: true } });
+    // @ts-expect-error this function exists!
+    await plugin.closeBundle();
+
+    expect(consoleErrorSpy).toHaveBeenCalledWith(
+      expect.stringContaining('[Source Maps Plugin] error while cleaning source map'),
+      expect.any(String),
+      expect.any(Error),
+    );
+  });
 });
 
 describe('_getUpdatedSourceMapSettings', () => {
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

);
await fs.promises.writeFile(mapFile, cleanedMapContent);
} catch {
// Map file doesn't exist, nothing to clean
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overly broad catch silently swallows non-ENOENT errors

Medium Severity

The new catch block silently swallows all errors, not just ENOENT (file not found). If readFile or writeFile fails due to permission errors, disk full, or other I/O issues, the failure is silently ignored. The try/catch block just above (lines 202–211) demonstrates the better pattern already used in this file: it checks whether the error is a known ENOENT and logs unexpected errors when debug is enabled. The same approach would be appropriate here.

Additional Locations (1)

Fix in Cursor Fix in Web

);
await fs.promises.writeFile(mapFile, cleanedMapContent);
} catch {
// Map file doesn't exist, nothing to clean
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix PR missing regression test for the change

Low Severity

This is a fix PR but no regression test was added to verify the race condition fix. The existing test file sourceMaps.test.ts has no tests covering the source map file cleaning logic (the readFile/writeFile path for .map files). A test verifying behavior when the map file doesn't exist — and ideally when other I/O errors occur — would help prevent regressions.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! Looks reasonable and pragmatic to me, thanks for fixing!

@chargome chargome merged commit 2b3ce34 into develop Mar 9, 2026
45 checks passed
@chargome chargome deleted the fix/sveltekit-sourcemap-toctou-race branch March 9, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants