Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 54 additions & 12 deletions pkg/rulemanager/cel/libraries/applicationprofile/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,11 @@ func (l *apLibrary) wasExecutedWithArgs(containerID, path, args ref.Val) ref.Val
return types.MaybeNoSuchOverloadErr(path)
}

// v1 limitation for rule authors: wasExecutedWithArgs is currently equivalent
// to wasExecuted — the args list is validated but not matched against. Any
// execution of the given path returns true regardless of its arguments. Full
// argument matching (ExecArgsByPath) will be added in a future version.
_ = args
if _, err := celparse.ParseList[string](args); err != nil {
// Parse the runtime args list from CEL. Empty list is valid ("exec'd
// with no args") and matches a profile entry whose Args is also empty
// or absent (empty profile Args = "no argv constraint").
runtimeArgs, err := celparse.ParseList[string](args)
if err != nil {
return types.NewErr("failed to parse args: %v", err)
}

Expand All @@ -84,20 +83,63 @@ func (l *apLibrary) wasExecutedWithArgs(containerID, path, args ref.Val) ref.Val
return types.Bool(true)
}

cp, _, err := profilehelper.GetProjectedContainerProfile(l.objectCache, containerIDStr)
if err != nil {
cp, _, perr := profilehelper.GetProjectedContainerProfile(l.objectCache, containerIDStr)
if perr != nil {
// Return a special error that will NOT be cached, allowing retry when profile becomes available.
// The caller should convert this to false after the cache layer.
return cache.NewProfileNotAvailableErr("%v", err)
return cache.NewProfileNotAvailableErr("%v", perr)
}

// Exact path match: walk the profile's Args for that path via
// CompareExecArgs (handles ⋯ single-arg and * zero-or-more tokens).
//
// ExecsByPath absent-vs-empty asymmetry — CodeRabbit upstream PR
// #807 finding #8. Three states to distinguish:
//
// 1. Path absent from cp.Execs.Values:
// Profile doesn't allow this exec at all → fall through to
// the pattern-match loop, then to false.
//
// 2. Path in Values, ABSENT from ExecsByPath (map lookup ok=false):
// Legacy / pre-args-projection profiles. Treated as
// "no argv constraint" — back-compat MATCH any args.
// This is the intentional fallback for profiles compiled
// against older storage versions that didn't populate the
// composite ExecsByPath surface.
//
// 3. Path in Values, PRESENT in ExecsByPath with an EMPTY arg
// list ([]):
// Profile explicitly captured "this path ran with no args".
// CompareExecArgs matches only when runtimeArgs is also
// empty. NOT a back-compat fallback — a deliberately tight
// constraint authored by the profile producer.
//
// The distinction matters for rule-author intuition: producing a
// signed profile that lists `{Path: /usr/bin/foo, Args: []}` is a
// CONSTRAINT, not a wildcard. Authors who want "any args" must
// omit the ExecsByPath entry (rare) or use an explicit `*`
// wildcard token in Args.
if _, ok := cp.Execs.Values[pathStr]; ok {
return types.Bool(true)
if profileArgs, ok := cp.ExecsByPath[pathStr]; ok {
if dynamicpathdetector.CompareExecArgs(profileArgs, runtimeArgs) {
return types.Bool(true)
}
} else {
// State 2: ExecsByPath absent → back-compat "no argv constraint".
return types.Bool(true)
}
}
// Check Patterns (dynamic-segment entries).
// Pattern path match: dynamic-segment paths in cp.Execs.Patterns.
// Args matching mirrors the exact-path case.
for _, execPath := range cp.Execs.Patterns {
if dynamicpathdetector.CompareDynamic(execPath, pathStr) {
return types.Bool(true)
if profileArgs, ok := cp.ExecsByPath[execPath]; ok {
if dynamicpathdetector.CompareExecArgs(profileArgs, runtimeArgs) {
return types.Bool(true)
}
} else {
return types.Bool(true)
}
}
}

Expand Down
134 changes: 130 additions & 4 deletions pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,14 @@ func TestExecWithArgsInProfile(t *testing.T) {
expectedResult: true,
},
{
// v1 degradation: args projection is out of scope; path-only matching.
// Args are anchored — wrong arg mismatch must reject the exec.
// Fork restores CompareExecArgs matching that upstream
// projection-v1 had temporarily dropped.
name: "Path matches but args don't match",
containerID: "test-container-id",
path: "/bin/ls",
args: []string{"-la", "/home"},
expectedResult: true,
expectedResult: false,
},
{
name: "Path doesn't exist",
Expand All @@ -229,12 +231,15 @@ func TestExecWithArgsInProfile(t *testing.T) {
expectedResult: true,
},
{
// v1 degradation: args projection is out of scope; path-only matching.
// /bin/ls in the profile has Args: ["-la", "/tmp"]. An empty
// runtime args list cannot satisfy a 2-arg anchored profile.
// (Empty profile Args = "no argv constraint" still matches via
// the back-compat branch; that's a separate case.)
name: "Empty args list",
containerID: "test-container-id",
path: "/bin/ls",
args: []string{},
expectedResult: true,
expectedResult: false,
},
}

Expand Down Expand Up @@ -301,6 +306,127 @@ func TestExecWithArgsNoProfile(t *testing.T) {
assert.False(t, actualResult, "ap.was_executed_with_args should return false when no profile is available")
}

// TestExecWithArgsWildcardInProfile exercises wildcard tokens inside a
// user-defined ApplicationProfile's exec arg vector:
//
// "⋯" (DynamicIdentifier) — matches exactly one argument position.
// "*" (WildcardIdentifier) — matches zero or more consecutive args.
//
// The runtime exec arg vector is matched against the profile via
// dynamicpathdetector.CompareExecArgs (added in
// k8sstormcenter/storage#23 — the matcher that this CEL function now
// routes through instead of slices.Compare).
func TestExecWithArgsWildcardInProfile(t *testing.T) {
objCache := objectcachev1.RuleObjectCacheMock{
ContainerIDToSharedData: maps.NewSafeMap[string, *objectcache.WatchedContainerData](),
}

objCache.SetSharedContainerData("test-container-id", &objectcache.WatchedContainerData{
ContainerType: objectcache.Container,
ContainerInfos: map[objectcache.ContainerType][]objectcache.ContainerInfo{
objectcache.Container: {
{
Name: "test-container",
},
},
},
})

profile := &v1beta1.ApplicationProfile{}
profile.Spec.Containers = append(profile.Spec.Containers, v1beta1.ApplicationProfileContainer{
Name: "test-container",
Execs: []v1beta1.ExecCalls{
// curl any URL: --user must be literal, value is one position.
{
Path: "/usr/bin/curl",
Args: []string{"--user", "⋯"},
},
// sh -c with any trailing payload (zero or more args).
{
Path: "/bin/sh",
Args: []string{"-c", "*"},
},
// ls -l in any directory — single trailing position.
{
Path: "/bin/ls",
Args: []string{"-l", "⋯"},
},
// echo with any number of greeting words after a literal anchor.
{
Path: "/bin/echo",
Args: []string{"hello", "*"},
},
},
})
objCache.SetApplicationProfile(profile)

env, err := cel.NewEnv(
cel.Variable("containerID", cel.StringType),
cel.Variable("path", cel.StringType),
cel.Variable("args", cel.ListType(cel.StringType)),
AP(&objCache, config.Config{}),
)
if err != nil {
t.Fatalf("failed to create env: %v", err)
}

testCases := []struct {
name string
path string
args []string
expectedResult bool
}{
// curl with --user, dynamic value
{"curl --user alice — ⋯ matches one arg", "/usr/bin/curl", []string{"--user", "alice"}, true},
{"curl --user alice bob — extra arg, ⋯ rejects", "/usr/bin/curl", []string{"--user", "alice", "bob"}, false},
{"curl --user — missing value, ⋯ requires one arg", "/usr/bin/curl", []string{"--user"}, false},
{"curl --pass alice — literal mismatch", "/usr/bin/curl", []string{"--pass", "alice"}, false},

// sh -c with arbitrary trailing payload
{"sh -c with single command", "/bin/sh", []string{"-c", "echo hi"}, true},
{"sh -c with multi-token command", "/bin/sh", []string{"-c", "while", "true;", "do", "sleep", "1;", "done"}, true},
{"sh -c with no trailing args (* matches zero)", "/bin/sh", []string{"-c"}, true},
{"sh -x — wrong flag", "/bin/sh", []string{"-x", "echo hi"}, false},

// ls -l in any directory
{"ls -l /var/log", "/bin/ls", []string{"-l", "/var/log"}, true},
{"ls -l with no directory (⋯ requires one)", "/bin/ls", []string{"-l"}, false},

// echo hello *
{"echo hello world from test", "/bin/echo", []string{"hello", "world", "from", "test"}, true},
{"echo hello (no trailing args)", "/bin/echo", []string{"hello"}, true},
{"echo goodbye world — wrong literal anchor", "/bin/echo", []string{"goodbye", "world"}, false},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ast, issues := env.Compile(`ap.was_executed_with_args(containerID, path, args)`)
if issues != nil {
t.Fatalf("failed to compile expression: %v", issues.Err())
}

program, err := env.Program(ast)
if err != nil {
t.Fatalf("failed to create program: %v", err)
}

result, _, err := program.Eval(map[string]interface{}{
"containerID": "test-container-id",
"path": tc.path,
"args": tc.args,
})
if err != nil {
t.Fatalf("failed to eval program: %v", err)
}

actualResult := result.Value().(bool)
assert.Equal(t, tc.expectedResult, actualResult,
"runtime args %v vs profile (one of curl/sh/ls/echo overlay): got %v want %v",
tc.args, actualResult, tc.expectedResult)
})
}
}

func TestExecWithArgsCompilation(t *testing.T) {
objCache := objectcachev1.RuleObjectCacheMock{}

Expand Down
36 changes: 24 additions & 12 deletions pkg/rulemanager/cel/libraries/applicationprofile/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ func (l *apLibrary) wasPathOpened(containerID, path ref.Val) ref.Val {
return types.Bool(false)
}

// wasPathOpenedWithFlags answers whether the projected ApplicationProfile
// contains an open-entry whose path matches the given path. The flags
// argument is parsed and validated for shape but is not used for matching
// in v1 — the OpenFlagsByPath projection slice is out of scope for v1
// (composite-key projection would balloon the cache footprint). When the
// flags-projection slice is added in a future spec revision, this helper
// becomes the path-AND-flag matcher and v1 callers continue to work.
func (l *apLibrary) wasPathOpenedWithFlags(containerID, path, flags ref.Val) ref.Val {
if l.objectCache == nil {
return types.NewErr("objectCache is nil")
Expand Down Expand Up @@ -105,17 +112,21 @@ func (l *apLibrary) wasPathOpenedWithSuffix(containerID, suffix ref.Val) ref.Val
}

if cp.Opens.All {
// All entries retained — scan to check for the suffix.
// All entries retained (no rule declared SuffixHits-style
// projection). Scan ONLY concrete entries in Values — Patterns
// contain wildcard tokens ('*' / '⋯') whose text doesn't safely
// answer suffix questions. CodeRabbit PR #43 open.go:79: a
// retained Pattern like "/var/log/pods/*/volumes/..." doesn't
// end with the concrete suffix "foo.log", but the concrete open
// it stands in for might — strings.HasSuffix on the pattern
// text returns false and produces a false negative. Patterns
// are inherently wildcard-shaped; concrete-path semantics live
// in Values (and in SuffixHits when projection is active).
for openPath := range cp.Opens.Values {
if strings.HasSuffix(openPath, suffixStr) {
return types.Bool(true)
}
}
for _, openPath := range cp.Opens.Patterns {
if strings.HasSuffix(openPath, suffixStr) {
return types.Bool(true)
}
}
return types.Bool(false)
}
// Projection applied — SuffixHits is authoritative; absent key = undeclared.
Expand Down Expand Up @@ -149,17 +160,18 @@ func (l *apLibrary) wasPathOpenedWithPrefix(containerID, prefix ref.Val) ref.Val
}

if cp.Opens.All {
// All entries retained — scan to check for the prefix.
// All entries retained — scan ONLY Values (concrete paths).
// Patterns contain wildcard tokens whose text doesn't safely
// answer prefix questions; a pattern starting with "/var/⋯/log"
// matches concrete paths starting with "/var/anything/log" but
// strings.HasPrefix against the pattern text returns false for
// "/var/foo/log...". Same fix as wasPathOpenedWithSuffix above.
// CodeRabbit PR #43 open.go:79 (Also applies to 111-123).
for openPath := range cp.Opens.Values {
if strings.HasPrefix(openPath, prefixStr) {
return types.Bool(true)
}
}
for _, openPath := range cp.Opens.Patterns {
if strings.HasPrefix(openPath, prefixStr) {
return types.Bool(true)
}
}
return types.Bool(false)
}
// Projection applied — PrefixHits is authoritative; absent key = undeclared.
Expand Down
Loading