fix(providers): analyze all FROM lines in multi-stage Dockerfiles via batch analysis#577
Conversation
… batch analysis Change the Dockerfile/Containerfile provider to parse every FROM instruction instead of only the last one. Each image gets its own SBOM in a purl-keyed map that is routed to the /api/v5/batch-analysis endpoint via requestStackBatch(). - Rename parseFromImage() to parseAllFromImages() returning an array - Build sbomByPurl map in getImageSBOM() following requestImages() pattern - Add batch flag to provider result for routing in analysis.js - Resolve ARG substitutions using declared defaults (matching VS Code extension) - FROM lines with unresolvable ARGs (no default) are skipped without failing Implements TC-5070 Assisted-by: Claude Code
Reviewer's GuideUpdates the Dockerfile/Containerfile OCI provider to parse and analyze all FROM stages in multi-stage Dockerfiles, resolve ARG-based image references using defaults, and emit batch SBOM results that are routed through new batch-handling paths in the analysis pipeline. Sequence diagram for batch Dockerfile stack analysissequenceDiagram
actor User
participant CLI as requestStack
participant Provider as oci_dockerfile_provider
participant Batch as requestStackBatch
User->>CLI: invoke requestStack(manifest, url, opts)
CLI->>Provider: provideStack(manifest, opts)
Provider-->>CLI: { content, contentType, ecosystem, batch:true }
CLI->>Batch: requestStackBatch(JSON.parse(content), url, html, opts)
Batch-->>User: batch analysis result
Flow diagram for parsing all FROM images with ARG resolutionflowchart TD
A[parseAllFromImages]
A-->B[getParser]
A-->C[getFromQuery]
A-->D[getArgQuery]
B & C & D -->E[parser.parse]
E-->F[collectArgs]
F-->G[iterate FROM matches]
G-->H{containsExpansion}
H-->|no|I[add imageSpec.text to images]
H-->|yes|J[resolveArgs]
J-->K{resolved != null}
K-->|yes|L[add resolved to images]
K-->|no|G
I-->M{images.length > 0}
L-->M
M-->|yes|N[return images]
M-->|no|O[throw error]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
getImageSBOM, the same image could appear in multiple FROM stages and will trigger multiplegenerateImageSBOMcalls even though thesbomByPurlentry is overwritten; consider de‑duplicating by purl or image ref to avoid redundant SBOM generation work. - When
parseAllFromImagesends up skipping all FROM lines due to unresolved ARGs, the thrown error is quite generic; including the list of unresolved variables or at least the problematic FROM lines would make it easier to diagnose why the Dockerfile could not be resolved.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `getImageSBOM`, the same image could appear in multiple FROM stages and will trigger multiple `generateImageSBOM` calls even though the `sbomByPurl` entry is overwritten; consider de‑duplicating by purl or image ref to avoid redundant SBOM generation work.
- When `parseAllFromImages` ends up skipping all FROM lines due to unresolved ARGs, the thrown error is quite generic; including the list of unresolved variables or at least the problematic FROM lines would make it easier to diagnose why the Dockerfile could not be resolved.
## Individual Comments
### Comment 1
<location path="src/providers/containerfile_parser.js" line_range="25" />
<code_context>
+
+export async function getArgQuery() {
+ const language = await init();
+ return new Query(language, '(arg_instruction (arg_pair name: (unquoted_string) @name default: (unquoted_string) @default))');
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** ARG query only captures unquoted defaults, missing valid quoted ARG defaults.
By restricting both `name` and `default` to `unquoted_string`, this misses valid cases like `ARG FOO="bar"` (or other representations tree-sitter may use). Those ARGs will be invisible to `collectArgs`, causing FROM lines that depend on them to be treated as unresolvable and skipped. Please broaden the matched node types for `default` (and possibly `name`) so quoted and alternative forms are included.
Suggested implementation:
```javascript
export async function getArgQuery() {
const language = await init();
return new Query(
language,
'(arg_instruction (arg_pair name: (_) @name default: (_) @default))',
);
}
```
If downstream code assumes `@name` and `@default` are always `unquoted_string` nodes, you may need to adjust it to handle other node types (e.g., string literals) by inspecting the captured nodes and normalizing their text (stripping quotes where needed).
</issue_to_address>
### Comment 2
<location path="test/providers/oci_dockerfile.test.js" line_range="65-66" />
<code_context>
+ expect(result).to.deep.equal(['node:18', 'nginx:alpine'])
+ })
+
+ /** Verifies that FROM lines with ARG substitution are resolved using declared defaults. */
+ test('resolves ARG substitution in FROM lines using declared defaults', async () => {
+ const content = [
+ 'ARG BASE_IMAGE=ubuntu:22.04',
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case covering `$VAR` (non-braced) substitutions in FROM to exercise both resolveArgs patterns
Currently only the `${VAR}` path in `resolveArgs` is covered by tests. To also exercise the `$VAR` branch of the regex, please add a similar test using `$BASE_IMAGE` in a FROM line, e.g.:
```js
test('resolves $VAR substitution in FROM lines using declared defaults', async () => {
const content = [
'ARG BASE_IMAGE=ubuntu:22.04',
'FROM $BASE_IMAGE AS base',
'FROM alpine:3.18',
].join('\n')
const result = await parseAllFromImages(content)
expect(result).to.deep.equal(['ubuntu:22.04', 'alpine:3.18'])
})
```
This ensures the `$VAR` pattern remains covered and prevents regressions on that path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #577 +/- ##
==========================================
+ Coverage 90.70% 91.01% +0.30%
==========================================
Files 38 38
Lines 7921 8001 +80
Branches 1373 1395 +22
==========================================
+ Hits 7185 7282 +97
+ Misses 736 719 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review — Classified as suggestion (2 items):
|
Verification Report for TC-5070 (commit df1dfe6)
Overall: WARNScope Containment details:
Repetitive Test Detection details: Several This comment was AI-generated by sdlc-workflow/verify-pr v0.12.2. |
Strum355
left a comment
There was a problem hiding this comment.
Some small changes but LGTM otherwise
…BOMs Broaden the tree-sitter ARG query to match any node type (not just unquoted_string) so quoted defaults like ARG FOO="bar" are captured. Strip surrounding quotes in collectArgs. Also skip redundant generateImageSBOM calls when the same image appears in multiple FROM lines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
/api/v5/batch-analysisChanges
src/providers/oci_dockerfile.js— renameparseFromImage()→parseAllFromImages(), buildsbomByPurlmap, addbatch: trueflagsrc/providers/containerfile_parser.js— addgetArgQuery()for ARG instruction parsingsrc/analysis.js— detect batch provider results and route torequestStackBatch()test/providers/oci_dockerfile.test.js— update tests for array return, add ARG resolution and batch output testsImplements TC-5070
Test plan
parseAllFromImagesreturns array of all image refsnpm test— 496 passing, 0 new failuresnpm run lint— 0 errors🤖 Generated with Claude Code
Summary by Sourcery
Analyze all FROM instructions in Dockerfiles for OCI image SBOM generation and route batch SBOM results through stack/component analysis.
New Features:
Enhancements:
Tests: