fix: CN-272 whiteout regex bug by parker-snyk · Pull Request #690 · snyk/snyk-docker-plugin

parker-snyk · 2025-08-08T19:03:27Z

We were using .wh. to look for whited out files, but we didn't escape the ., so it was a match-any character

We also weren't following the OpenContainer spec: https://github.com/opencontainers/image-spec/blob/main/layer.md#whiteouts

.wh. needs to be the prefix of the filename. So it either needs to prefix everything (no slashes, just filename) or it needs to prefix the filename after the last slash (full path)

Our current implementation just looks for .wh. to appear anywhere, so we are incorrectly whiting out files. This includes libraries, so all of the node 'which' packages were being skipped, for example

lib/extractor/index.ts

lib/analyzer/image-inspector.ts

test/lib/extractor/index.spec.ts

lib/extractor/index.ts

bgardiner · 2025-08-22T01:37:58Z

lib/extractor/index.ts

+export function removeWhiteoutPrefix(filename: string): string {
+  // Replace .wh. that appears at the start or after the last slash,
+  // and ensure no slashes come after .wh.
+  return filename.replace(/^(.*\/)?\.wh\.([^\/]*)$/, "$1$2");


do you think this should be "+" instead? I guess we expect 0 times to be a bug in the generator (i.e., impossible) but we should still remove it?

(The comment applied to the * in [^\/]*)

I went with * intentionally to handle the degenerate case where someone has a file literally named .wh. with nothing after it. In practice I doubt we'd ever see it, but figured it's safer to handle it than to silently skip it.

Happy to switch to + if you feel strongly though.

bgardiner · 2025-08-22T01:40:11Z

lib/extractor/index.ts

+ */
 export function isWhitedOutFile(filename: string) {
-  return filename.match(/.wh./gm);
+  const lastSlashIndex = filename.lastIndexOf("/");


this assumes unix path separator -- do we need to handle windows too?

I just updated to use path.basename so it should account for this now - https://nodejs.org/api/path.html#windows-vs-posix
I didn't know about this function until you mentioned it, so let me know if I'm incorrect

You're right, path.basename only splits on the platform's native separator, so on Linux it wouldn't handle backslashes. I extracted a getBasename helper that splits on both / and \ regardless of platform, and isWhitedOutFile and isOpaqueWhiteout both use it now

bgardiner · 2025-08-22T01:50:20Z

lib/extractor/index.ts

+ * https://www.madebymikal.com/interpreting-whiteout-files-in-docker-image-layers
+ * https://github.com/opencontainers/image-spec/blob/main/layer.md#whiteouts
+ */
 export function isWhitedOutFile(filename: string) {


does this function also need to account for opaque whiteouts: https://github.com/opencontainers/image-spec/blob/main/layer.md#opaque-whiteout?

In other words, check that the filename is not .wh..wh..opq? Looking above, it's not clear our extractor logic even handles opaque whiteouts at all 🤔

I think as long as it starts with .wh it should account for it, it looks like out logic checks if a file is or within any whited out file or directory, so if a directory starts with .wh, any files under it should be excluded. I can manually verify though

Okay, I misunderstood how .wh..wh..opq worked, I'll fix this

You were right, I didn't realize opaque means "nuke everything in this directory from lower layers." I've pushed a fix that adds isOpaqueWhiteout() detection and tracks opaque directories during layer merging so files from older layers in those dirs get properly excluded. Tests included

Refactor isWhitedOutFile to use cross-platform path.basename and update removeWhiteoutPrefix regex to support both POSIX and Windows path separators. This resolves a regression introduced by the initial whiteout regex fix where Windows-style paths would not be correctly identified or parsed.

…me handling - Handle .wh..wh..opq opaque whiteouts per OCI image spec: files in opaque-whiteout directories from older layers are now properly excluded - Replace path.basename with a custom getBasename helper that handles both / and \ separators regardless of platform, keeping isWhitedOutFile and isOpaqueWhiteout consistent with removeWhiteoutPrefix - Add tests for isOpaqueWhiteout

Opaque whiteouts should not affect files from the same layer — only files from older layers. Collecting opaque dirs in a per-layer set and merging after the inner loop prevents new files in the same layer from being incorrectly excluded.

isFileInOpaqueDir was using path.dirname and path.sep directly, which breaks on Windows where path.sep is \ but Docker paths use /. Reuse the existing isFileInFolder helper which normalizes paths first — same pattern as isFileInARemovedFolder.

snyk-pr-review-bot · 2026-04-06T21:06:30Z

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Platform Incompatibility 🟠 [major] The `isFileInFolder` function (used via `isFileInOpaqueDir`) uses `path.sep` and `path.join`. Since container image layers always use POSIX paths (`/`), this logic will fail on Windows hosts because `path.join` will insert backslashes (`\`), making `startsWith` return false for all nested files. The code should use a hardcoded `/` or `path.posix` for image path manipulation. isFileInFolder(filename, opaqueDir), Regex Logic Error 🟡 [minor] The regex in `removeWhiteoutPrefix` uses `^(.[\/\\])?`, where the `?` makes the path prefix optional. However, the subsequent `\.wh\.` expects a literal dot. In the root directory (e.g., `.wh.file`), the first capture group `$1` is empty. The replacement `$1$2` will work for root files, but the unit tests for root files (`.wh.hosts` -> `hosts`) only pass because the regex matches; if the path contains multiple `.wh.` sequences, the greedy `.` might behave unexpectedly compared to the standard `getBasename` logic used in detection. return filename.replace(/^(.[\/\\])?\.wh\.([^\/\\])$/, "$1$2");
📚 Repository Context Analyzed This review considered 18 relevant code sections from 14 files (average relevance: 0.96) lib/extractor/oci-archive/layer.ts (2 segments, lines 204-481) lib/extractor/docker-archive/layer.ts (lines 1-136) lib/extractor/kaniko-archive/layer.ts (lines 1-134) lib/analyzer/static-analyzer.ts (lines 1-89) lib/extractor/types.ts (lines 1-157) lib/inputs/node/static.ts (lines 1-49) lib/inputs/file-pattern/static.ts (lines 1-62) ... and 7 more file(s)

parker-snyk force-pushed the CN-272-whiteout-regex-bug branch 3 times, most recently from 7f7dd67 to f59fe4a Compare August 8, 2025 19:30

parker-snyk marked this pull request as ready for review August 8, 2025 19:34

parker-snyk requested a review from a team as a code owner August 8, 2025 19:34

parker-snyk requested review from bgardiner and mtstanley-snyk August 8, 2025 19:34

parker-snyk enabled auto-merge (squash) August 8, 2025 19:35

bgardiner reviewed Aug 8, 2025

View reviewed changes

lib/extractor/index.ts Outdated Show resolved Hide resolved

lib/extractor/index.ts Outdated Show resolved Hide resolved

lib/analyzer/image-inspector.ts Show resolved Hide resolved

test/lib/extractor/index.spec.ts Show resolved Hide resolved

parker-snyk disabled auto-merge August 8, 2025 20:37

This was referenced Aug 11, 2025

chore: code cleanup (CN-271) #692

Closed

chore: code cleanup CN-276 #693

Merged

This comment was marked as outdated.

Sign in to view

parker-snyk added 12 commits August 18, 2025 14:16

fix: .test is faster than .match regex

bd2c845

chore: we don't need to keep looping

6064ac0

chore: unit tests

cb14317

chore: improve readability / consistency

4f66915

chore: formatting

008a8ea

chore: revert out of scope files

6af7adf

fix: update check to follow .wh. specs

6982327

fix: update replace regex

9670836

fix: regex

84a7096

chore: linting

dc2517f

fix: the snapshots were not expecting any names with wh in them

b8a9b38

chore: tests

ef6a033

parker-snyk force-pushed the CN-272-whiteout-regex-bug branch from ea113ba to ef6a033 Compare August 18, 2025 18:18

parker-snyk added 3 commits August 18, 2025 14:19

chore: revert

e6aa931

chore: revert

12c304f

fix: fix the shas for snapshot

6a4a4c5

parker-snyk added 3 commits August 18, 2025 15:47

fix: some snapshot changes

1a0c472

fix: more snapshot changes

e47f47a

chore: update snapshots

17d541d

parker-snyk requested a review from bgardiner August 19, 2025 13:37

bgardiner reviewed Aug 22, 2025

View reviewed changes

parker-snyk marked this pull request as draft November 7, 2025 20:45

parker-snyk added 4 commits April 6, 2026 16:05

fix: whiteout regex bug (CN-272)

f46d29b

parker-snyk marked this pull request as ready for review April 6, 2026 20:34

This comment has been minimized.

Sign in to view

parker-snyk force-pushed the CN-272-whiteout-regex-bug branch from 05f6f4a to 3e278f7 Compare April 6, 2026 20:54

This comment has been minimized.

Sign in to view

parker-snyk force-pushed the CN-272-whiteout-regex-bug branch from 3e278f7 to 40159a2 Compare April 6, 2026 21:01

parker-snyk requested a review from bgardiner April 6, 2026 21:01

Conversation

parker-snyk commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parker-snyk Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parker-snyk Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

snyk-pr-review-bot bot commented Apr 6, 2026

PR Reviewer Guide 🔍

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

parker-snyk commented Aug 8, 2025 •

edited

Loading

parker-snyk Aug 27, 2025 •

edited

Loading

parker-snyk Apr 6, 2026 •

edited

Loading