From 6d984254ca663aff8af4d1cd8ec618730adb4205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 25 Jun 2026 09:42:48 +0200 Subject: [PATCH] Make sure to always sanitize source contents even when no PII sanitization is requested This was a regression from #6018. Previously, we didn't have a content column in the sources table. But we now added it, and they are filled automatically during the source map resolution. We would like to add a sharing feature soon, but this needs to be an explicit approval. Due to the early return in this `sanitizePII` function, we were mistakenly keeping these content values if we hit the early return (if the user wants to sanitize nothing). But even if the user wants to upload everything, we should unconditionally sanitize the sources. --- src/profile-logic/sanitize.ts | 27 ++++++++++++++++----------- src/test/unit/sanitize.test.ts | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/profile-logic/sanitize.ts b/src/profile-logic/sanitize.ts index a69bec579f..851f919f7f 100644 --- a/src/profile-logic/sanitize.ts +++ b/src/profile-logic/sanitize.ts @@ -61,10 +61,23 @@ export function sanitizePII( maybePIIToBeRemoved: RemoveProfileInformation | null, markerSchemaByName: MarkerSchemaByName ): SanitizeProfileResult { + // FIXME: Currently we always sanitize the source contents while publishing. + // But we should have a sharing option in the publish panel to be able to + // include sources. This needs to happen before the early return below, so + // that the source contents are removed even when no other PII removal is + // requested. + const unconditionallySanitizedShared = { + ...profile.shared, + sources: { + ...profile.shared.sources, + content: Array(profile.shared.sources.content.length).fill(null), + }, + }; + if (maybePIIToBeRemoved === null) { - // Nothing is sanitized. + // Nothing else is sanitized, but the source contents are still removed. return { - profile, + profile: { ...profile, shared: unconditionallySanitizedShared }, isSanitized: false, translationMaps: null, committedRanges: null, @@ -122,18 +135,10 @@ export function sanitizePII( } const stringTable = StringTable.withBackingArray(stringArray); const newShared = { - ...profile.shared, + ...unconditionallySanitizedShared, stringArray, }; - // FIXME: Currently we always sanitize the source contents while publishing. - // But we should have a sharing option in the publish panel to be able to - // include sources. - newShared.sources = { - ...newShared.sources, - content: Array(newShared.sources.content.length).fill(null), - }; - let stackFlags: Uint8Array | null = null; if (windowIdFromPrivateBrowsing.size > 0) { diff --git a/src/test/unit/sanitize.test.ts b/src/test/unit/sanitize.test.ts index 70d43c9ba6..c22f1ad41b 100644 --- a/src/test/unit/sanitize.test.ts +++ b/src/test/unit/sanitize.test.ts @@ -1275,4 +1275,21 @@ describe('sanitizePII', function () { 'file2.js' ); }); + + it('always removes source contents even when no PII removal is requested', function () { + const { profile } = getProfileFromTextSamples(`A[file:file1.js]`); + // There should be only one source. + expect(profile.shared.sources.content.length).toEqual(1); + // Pretend the source content have been loaded. + profile.shared.sources.content[0] = 'source code'; + + // A null `RemoveProfileInformation` means the user kept all sharing options + // checked, so no other sanitization happens. + const { profile: sanitizedProfile } = sanitizePII(profile, [], null, {}); + + expect(sanitizedProfile.shared.sources.content.length).toEqual( + profile.shared.sources.content.length + ); + expect(sanitizedProfile.shared.sources.content[0]).toBe(null); + }); });