Skip to content

chore: remove allowNewContainerFacts feature flag - #7143

Merged
kateeselius merged 3 commits into
mainfrom
chore/remove-container-new-facts-ff
Aug 18, 2026
Merged

chore: remove allowNewContainerFacts feature flag#7143
kateeselius merged 3 commits into
mainfrom
chore/remove-container-new-facts-ff

Conversation

@kateeselius

Copy link
Copy Markdown
Contributor

What this does

Removes the allowNewContainerFacts feature flag (constant CONTAINER_NEW_FACTS_FEATURE_FLAG) from the container test/monitor flow. The flag is 100% rolled out, so all facts returned by snyk-docker-plugin are now passed through unfiltered.

Removed:

  • CONTAINER_NEW_FACTS_FEATURE_FLAG constant in src/cli/commands/constants.ts
  • filterDockerFacts helper, the FILTERED_FACT_TYPES / ADDITIONAL_FILTERED_FACT_TYPES_FOR_APP_PROJECTS sets, and the shouldFilterFact internal in src/lib/ecosystems/common.ts
  • The filterDockerFacts calls at the three call sites (snyk-test/assemble-payloads.ts, ecosystems/monitor.ts, ecosystems/test.ts)
  • test/jest/unit/ecosystems-common.spec.ts (its purpose was to test the removed behavior)

Notes for the reviewer

  • isUnmanagedEcosystem in common.ts is preserved; only the docker-fact filtering was tied to the flag.
  • ESLint clean on the touched files.

More information

The allowNewContainerFacts flag previously gated whether new fact types (containerConfig, history, pluginVersion, platform, pluginWarnings, imageNames, ociDistributionMetadata) were included in the CLI's payloads to the Snyk API. Now that it's fully rolled out, the filtering path is dead code.

@snyk-io

snyk-io Bot commented Aug 17, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?

⚠️ There are multiple commits on your branch, please squash them locally before merging!

Generated by 🚫 dangerJS against c068bfa

@kateeselius
kateeselius marked this pull request as ready for review August 17, 2026 19:19
@kateeselius
kateeselius requested a review from a team as a code owner August 17, 2026 19:19
@snyk-pr-review-bot

This comment has been minimized.

@kateeselius
kateeselius enabled auto-merge August 18, 2026 12:01
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

ReferenceError/TypeError 🔴 [critical]

In monitorEcosystem, the logic to apply policy (lines 73-78) attempts to iterate over scanResultsByPath[path]. However, in the new code, scanResultsByPath[path] is initialized only at line 71, but the code at line 76 (visible in context) expects the array to exist to call forEach. If pluginResponse.scanResults is undefined or if the initialization is bypassed by an error, this will crash. More importantly, the original code initialized the path entry before the policy block, but the new structure might lead to accessing an uninitialized property if the plugin returns no results for a path.

scanResultsByPath[path] = pluginResponse.scanResults;
📚 Repository Context Analyzed

This review considered 11 relevant code sections from 10 files (average relevance: 0.59)

🤖 Repository instructions applied (from AGENTS.md)

@kateeselius
kateeselius merged commit 7e19804 into main Aug 18, 2026
9 checks passed
@kateeselius
kateeselius deleted the chore/remove-container-new-facts-ff branch August 18, 2026 13:51
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.

3 participants