[entlein]

exec-path: symlink-faithful argv[0] resolution

⚠️ Merge order

This PR depends on #323 and must be merged AFTER it.

PR #323 defines WildcardIdentifier (and the broader CollapseConfig / NewPathAnalyzerWithConfigs surface) in types.go and analyzer.go. Without it, compare_exec_args.go (this PR) fails to compile because WildcardIdentifier is undefined.

Correct merge order:

1. #323 — path-wildcards: anchored trailing-* + per-endpoint port + R0040 args
2. #322 (this PR) — allowing wildcards in exec args

Summary by CodeRabbit

• New Features

  • Enhanced command-line argument pattern matching: anchored wildcard and single-value dynamic tokens, plus a new "args required" option that changes empty-vs-anchored behavior and is exposed in the API schema.

• Tests

  • Expanded coverage for literal, dynamic, wildcard, mixed patterns, realistic argv forms, argv[0] semantics, ReDoS resistance, back-compat checks, and protobuf roundtrip/field-tag tests.

• Chores

  • API conversion mappings updated to carry the new flag.

[coderabbitai]

Warning

Review limit reached

@entlein, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes and 59 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 83f49ab5-bd77-4eeb-9492-7a159a1bd0ab

📥 Commits

Reviewing files that changed from the base of the PR and between abde5c3 and 5e39d00.

⛔ Files ignored due to path filters (3)

• pkg/apis/softwarecomposition/v1beta1/generated.pb.go is excluded by !**/*.pb.go
• pkg/generated/applyconfiguration/softwarecomposition/v1beta1/execcalls.go is excluded by !**/generated/**
• pkg/generated/openapi/zz_generated.openapi.go is excluded by !**/generated/**

📒 Files selected for processing (7)

• pkg/apis/softwarecomposition/types.go
• pkg/apis/softwarecomposition/v1beta1/execcalls_protobuf_test.go
• pkg/apis/softwarecomposition/v1beta1/generated.proto
• pkg/apis/softwarecomposition/v1beta1/types.go
• pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go
• pkg/registry/file/dynamicpathdetector/compare_exec_args.go
• pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go

📝 Walkthrough

Walkthrough

Adds MatchExecArgs/CompareExecArgs with a memoized anchored wildcard matcher for profile vs runtime argv vectors, plus ExecCalls.ArgsRequired (types, proto, conversions) and tests covering matching semantics, wire compatibility, and DoS-resistance.

Changes

Exec argument matching with wildcard support

Layer / File(s)	Summary
Matcher implementation pkg/registry/file/dynamicpathdetector/compare_exec_args.go	Adds MatchExecArgs, CompareExecArgs, and matchExecArgsStrict: memoized recursive anchored matching where ⋯ matches exactly one arg, * matches zero-or-more, and literals require equality.
Matcher tests pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go	Adds extensive tests: literal matching, ⋯ semantics (including adjacency), * semantics (including absorption/anchoring), mixed tokens, realistic argv patterns, argv[0] bare-name vs full-path checks, ReDoS-resistance timing test, and MatchExecArgs contract/back-compat tests.

ExecCalls ArgsRequired API + proto

Layer / File(s)	Summary
Types and proto pkg/apis/softwarecomposition/types.go, pkg/apis/softwarecomposition/v1beta1/types.go, pkg/apis/softwarecomposition/v1beta1/generated.proto	Adds ArgsRequired bool to ExecCalls with documentation describing strict empty-args semantics and effect on matching.
Protobuf tests and conversions pkg/apis/softwarecomposition/v1beta1/execcalls_protobuf_test.go, pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go	Adds roundtrip and wire-tag tests for argsRequired and updates generated conversions to map the new field; includes a test helper for string-slice equality treating nil/empty equivalently.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant MatchExecArgs
  participant matchExecArgsStrict
  participant memoStore
  Caller->>MatchExecArgs: MatchExecArgs(profileArgs, argsRequired, runtimeArgs)
  MatchExecArgs->>matchExecArgsStrict: delegate when strict required
  matchExecArgsStrict->>memoStore: lookup (pi,ri)
  memoStore-->>matchExecArgsStrict: cached result / miss
  matchExecArgsStrict->>matchExecArgsStrict: recurse advance or try wildcard splits
  matchExecArgsStrict->>memoStore: store result(pi,ri)
  matchExecArgsStrict-->>MatchExecArgs: boolean
  MatchExecArgs-->>Caller: return boolean

Loading

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers:

• matthyx

"I hop through args with gentle care,
Stars and dots tidy up the air,
Memo remembers each tiny part,
Match and return with all my heart,
A rabbit's cheer for code made fair." 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'allowing wildcards in exec args' clearly and concisely describes the main feature addition in this changeset: support for wildcard matching in execution arguments.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

pkg/registry/file/dynamicpathdetector/compare_exec_args.go (1)

45-55: 💤 Low value

Collapse seen + memo into a single map using the comma-ok idiom.

Two parallel maps keyed by the same state double allocations and obscure intent. The standard Go pattern is to check presence on the result map directly. Behavior is identical.

♻️ Proposed simplification

 	type state struct{ pi, ri int }
-	memo := make(map[state]bool, (len(profileArgs)+1)*(len(runtimeArgs)+1))
-	seen := make(map[state]bool, (len(profileArgs)+1)*(len(runtimeArgs)+1))
+	memo := make(map[state]bool, (len(profileArgs)+1)*(len(runtimeArgs)+1))
 
 	var match func(pi, ri int) bool
 	match = func(pi, ri int) bool {
 		s := state{pi: pi, ri: ri}
-		if seen[s] {
-			return memo[s]
+		if v, ok := memo[s]; ok {
+			return v
 		}
-		seen[s] = true

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/registry/file/dynamicpathdetector/compare_exec_args.go` around lines 45 -
55, The code uses two maps (seen and memo) keyed by state in compare logic;
collapse them into a single map (e.g., memo map[state]bool) and use the comma-ok
idiom inside match to check if an entry exists: construct s := state{pi: pi, ri:
ri}, if val, ok := memo[s]; ok { return val } then continue computing and store
the result in memo[s] before returning. Update references to seen and memo in
the match closure and initialization (remove seen, keep memo sized with
(len(profileArgs)+1)*(len(runtimeArgs)+1)).

pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go (1)

300-313: 💤 Low value

Wall-clock budget is shared-runner flaky; consider a deterministic cap.

A 100ms wall-clock budget on adversarial input is a reasonable smoke test, but shared CI runners under load can blow past it without an actual algorithmic regression — producing flakes that erode trust in the suite. Options that retain the guarantee without time-sensitivity:

• Increase budget substantially (e.g., 1–2s) — still catches an exponential blow-up (which would be many seconds to minutes), with much wider headroom for noisy runners.
• Or, scale the wildcard count up (e.g., 30–40 *s) so the unmemoised form would take effectively forever, then use a generous budget. The memoised form stays microseconds either way.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go` around
lines 300 - 313, Replace the flaky 100ms wall-clock assertion in the test that
times CompareExecArgs by increasing the deterministic cap: change the elapsed
check to use a much larger budget (e.g., 2*time.Second) instead of
100*time.Millisecond (update the local variables start/elapsed check around
CompareExecArgs), or alternatively make the adversarial input stronger by
increasing the wildcard count (the pattern used to build the profile) so the
unmemoised algorithm would take orders of magnitude longer while keeping the
memoised form fast; update the test assertion and any comment to reflect the new
budget or stronger input.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/registry/file/dynamicpathdetector/compare_exec_args.go`:
- Line 66: The package references an undefined constant WildcardIdentifier (used
in compare_exec_args.go and tests) which causes compilation failure; add a
string constant named WildcardIdentifier (value "*" to represent "zero or more
consecutive arguments") next to the existing DynamicIdentifier constant in the
package's types definitions so all references to WildcardIdentifier resolve
correctly and behavior matches the documented wildcard semantics.

---

Nitpick comments:
In `@pkg/registry/file/dynamicpathdetector/compare_exec_args.go`:
- Around line 45-55: The code uses two maps (seen and memo) keyed by state in
compare logic; collapse them into a single map (e.g., memo map[state]bool) and
use the comma-ok idiom inside match to check if an entry exists: construct s :=
state{pi: pi, ri: ri}, if val, ok := memo[s]; ok { return val } then continue
computing and store the result in memo[s] before returning. Update references to
seen and memo in the match closure and initialization (remove seen, keep memo
sized with (len(profileArgs)+1)*(len(runtimeArgs)+1)).

In `@pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go`:
- Around line 300-313: Replace the flaky 100ms wall-clock assertion in the test
that times CompareExecArgs by increasing the deterministic cap: change the
elapsed check to use a much larger budget (e.g., 2*time.Second) instead of
100*time.Millisecond (update the local variables start/elapsed check around
CompareExecArgs), or alternatively make the adversarial input stronger by
increasing the wildcard count (the pattern used to build the profile) so the
unmemoised algorithm would take orders of magnitude longer while keeping the
memoised form fast; update the test assertion and any comment to reflect the new
budget or stronger input.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e50038fd-c79c-449f-b3e3-2d53a45b7e78

📥 Commits

Reviewing files that changed from the base of the PR and between 2a689d8 and 631bb46.

📒 Files selected for processing (2)

• pkg/registry/file/dynamicpathdetector/compare_exec_args.go
• pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go

[entlein]

@coderabbitai review

Collective re-review request after applying your atomic-split advisory + rabbit-feedback fixes across the full SBOB upstream PR set. Each PR is now scoped to a single tier per your dependency map:

Tier 0 (opens):

• path-wildcards: anchored trailing-* + per-endpoint port + R0040 args #323 (path-wildcards core: anchored trailing-*, per-port key, WildcardIdentifier + CollapseConfig API — squash per your "merge first / or squash together" note)
• feat(cel): was_path_opened* CEL helpers in applicationprofile library node-agent#811 (was_path_opened* CEL helpers — NEW)

Tier 2 (execs):

• allowing wildcards in exec args #322 (CompareExecArgs)
• profile-compaction: CollapseConfig CRD + projection overlay + user-maged lifecycle #325 (CollapseConfig CRD + processors)
• allowing wildcards in exec args node-agent#805 (resolveExecPath + 3-arg parse)
• path-wildcards: anchored trailing-* + per-endpoint port + R0040 args node-agent#807 (R0040 args consumer)
• feat(projection): add ExecsByPath composite-key surface to ProjectedContainerProfile node-agent#812 (ExecsByPath composite-key surface — NEW)

Tier 3 (network):

• allowing CIDRs, wildcards and Plural in IP and DNS #324 (networkmatch package)
• allowing CIDRs, wildcards and Plural in IP and DNS node-agent#806 (network wildcards CEL)

Tier 4 (tamper/sign):

• profile-compaction: CollapseConfig CRD + projection overlay + user-ma… node-agent#808 (projection overlay + tamper + non-blocking fanout + NN resource)
• sign and detect tampering node-agent#809 (cosign sign/verify + tamper detection)

Umbrellas:

• SBOB wildcards: DNS + IP + path exec wildcards, sign/tamper  #326
• SBOB wildcards: DNS + IP + path exec wildcards, sign/tamper  node-agent#810

Please confirm atomicity per your earlier advisory and flag any remaining cross-PR coupling we missed.

[coderabbitai]

@entlein: Kicking off a full review of this PR. I'll assess atomicity and flag any cross-PR coupling specific to #322 as part of the Tier 2 exec layer.

Re-review requested after applying atomic-split advisory and rabbit-feedback fixes. Please assess CompareExecArgs (PR #322) for correctness, completeness, and Tier 2 coupling against the dependency map (Tier 0: #323; Tier 2 peers: #325, node-agent#805, #807, #812).

✅ Actions performed

Full review triggered.

[matthyx]

Blocking: this contract currently can't work as documented. ExecCalls.Args is serialized as json:\"args,omitempty\", so an explicit args: [] round-trips back as omitted/nil. Combined with if len(profileArgs) == 0 { return true }, both cases become "no argv constraint", which means a profile can't reliably express "no args" and was_executed_with_args will silently accept unexpected argv.

Even changing this branch to profileArgs == nil would still leave the storage/API round-trip problem. I think this needs either a representation that can persist the distinction (pointer/explicit flag/etc.) or semantics that do not rely on distinguishing omitted from empty.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/apis/softwarecomposition/v1beta1/execcalls_protobuf_test.go`:
- Around line 7-13: The build breaks due to an undefined symbol
WildcardIdentifier referenced in compare_exec_args.go; fix by either importing
the package that actually defines WildcardIdentifier or (to keep this PR atomic)
add a minimal local shim for WildcardIdentifier in the same package so the code
compiles: define the symbol with the same kind (e.g., a struct or typed
constant) and the minimal methods/fields used by the code that calls it (look
for usages in functions like the exec-args comparison helpers in
compare_exec_args.go) so tests and typechecking pass until `#323` is merged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 70dee8f8-6315-4202-9e6e-735bef295b4a

📥 Commits

Reviewing files that changed from the base of the PR and between 2d02375 and abde5c3.

⛔ Files ignored due to path filters (3)

• pkg/apis/softwarecomposition/v1beta1/generated.pb.go is excluded by !**/*.pb.go
• pkg/generated/applyconfiguration/softwarecomposition/v1beta1/execcalls.go is excluded by !**/generated/**
• pkg/generated/openapi/zz_generated.openapi.go is excluded by !**/generated/**

📒 Files selected for processing (7)

• pkg/apis/softwarecomposition/types.go
• pkg/apis/softwarecomposition/v1beta1/execcalls_protobuf_test.go
• pkg/apis/softwarecomposition/v1beta1/generated.proto
• pkg/apis/softwarecomposition/v1beta1/types.go
• pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go
• pkg/registry/file/dynamicpathdetector/compare_exec_args.go
• pkg/registry/file/dynamicpathdetector/tests/compare_exec_args_test.go

✅ Files skipped from review due to trivial changes (1)

• pkg/apis/softwarecomposition/v1beta1/zz_generated.conversion.go

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

PR is still cross-coupled and non-atomic without #323.

Typecheck currently fails with undefined: WildcardIdentifier in pkg/registry/file/dynamicpathdetector/compare_exec_args.go (Line 113), which confirms this PR cannot compile independently.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/apis/softwarecomposition/v1beta1/execcalls_protobuf_test.go` around lines
7 - 13, The build breaks due to an undefined symbol WildcardIdentifier
referenced in compare_exec_args.go; fix by either importing the package that
actually defines WildcardIdentifier or (to keep this PR atomic) add a minimal
local shim for WildcardIdentifier in the same package so the code compiles:
define the symbol with the same kind (e.g., a struct or typed constant) and the
minimal methods/fields used by the code that calls it (look for usages in
functions like the exec-args comparison helpers in compare_exec_args.go) so
tests and typechecking pass until `#323` is merged.