Skip to content

fix(conda): avoid panic on an all-operator dependency line#10955

Open
arpitjain099 wants to merge 1 commit into
aquasecurity:mainfrom
arpitjain099:fix/conda-env-empty-dep
Open

fix(conda): avoid panic on an all-operator dependency line#10955
arpitjain099 wants to merge 1 commit into
aquasecurity:mainfrom
arpitjain099:fix/conda-env-empty-dep

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

parseDependency normalizes a conda environment.yml dependency string by turning the version operators into spaces, then splits on whitespace and reads parts[0]:

line = strings.NewReplacer(">", " >", "<", " <", "=", " ").Replace(line)
parts := strings.Fields(line)
name := parts[0]

If the dependency value is only operators, e.g. a stray "=", the replaced line is all whitespace and strings.Fields returns an empty slice, so parts[0] panics with an index-out-of-range and aborts the scan.

Repro (environment.yml):

name: crash
dependencies:
  - "="

"=" is a valid YAML string scalar, so it reaches parseDependency. The sibling text parsers (golang/sum, ruby/bundler, rust/cargo) all guard the empty-split case before indexing; conda did not.

The fix returns an empty name/version for such a line, which the caller already drops via its existing if pkg.Name == "" { continue }. go test ./pkg/dependency/parser/conda/... passes.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>

@DmitriyLewen DmitriyLewen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello @arpitjain099
Thanks for the fix — it's correct and minimal. One request before merging: please add a regression test for the fixed path.

Right now the panicking input isn't covered by any case, so if the guard is ever accidentally removed or broken, the tests won't catch it and the panic will come back.

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