From 5c0bba0b4fce798049cc61d32d0a337d9d83c078 Mon Sep 17 00:00:00 2001 From: entlein Date: Fri, 15 May 2026 22:32:52 +0200 Subject: [PATCH 1/3] path-wildcards: anchored trailing-* + per-endpoint port + R0040 args Signed-off-by: entlein --- .../cel/libraries/applicationprofile/ap.go | 22 -- .../cel/libraries/applicationprofile/exec.go | 40 ++- .../libraries/applicationprofile/exec_test.go | 134 ++++++++- .../applicationprofile/integration_test.go | 2 +- .../cel/libraries/applicationprofile/open.go | 69 ++--- .../libraries/applicationprofile/open_test.go | 284 ++++++------------ 6 files changed, 272 insertions(+), 279 deletions(-) diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go index ce86d7ab88..fabf311c2e 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go @@ -111,25 +111,6 @@ func (l *apLibrary) Declarations() map[string][]cel.FunctionOpt { }), ), }, - "ap.was_path_opened_with_flags": { - cel.Overload( - "ap_was_path_opened_with_flags", []*cel.Type{cel.StringType, cel.StringType, cel.ListType(cel.StringType)}, cel.BoolType, - cel.FunctionBinding(func(values ...ref.Val) ref.Val { - if len(values) != 3 { - return types.NewErr("expected 3 arguments, got %d", len(values)) - } - if l.detailedMetrics && l.metrics != nil { - l.metrics.IncHelperCall("ap.was_path_opened_with_flags") - } - wrapperFunc := func(args ...ref.Val) ref.Val { - return l.wasPathOpenedWithFlags(args[0], args[1], args[2]) - } - cachedFunc := l.functionCache.WithCache(wrapperFunc, "ap.was_path_opened_with_flags", cache.HashForContainerProfile(l.objectCache)) - result := cachedFunc(values[0], values[1], values[2]) - return cache.ConvertProfileNotAvailableErrToBool(result, false) - }), - ), - }, "ap.was_path_opened_with_suffix": { cel.Overload( "ap_was_path_opened_with_suffix", []*cel.Type{cel.StringType, cel.StringType}, cel.BoolType, @@ -354,9 +335,6 @@ func (e *apCostEstimator) EstimateCallCost(function, overloadID string, target * case "ap.was_path_opened": // Cache lookup + O(n) linear search + dynamic path comparison cost = 25 - case "ap.was_path_opened_with_flags": - // Cache lookup + O(n) search + dynamic path comparison + O(f*p) flag comparison - cost = 40 case "ap.was_path_opened_with_suffix": // Cache lookup + O(n) linear search + O(n*len(suffix)) string suffix checks cost = 20 diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go index b69a69c0ea..5f57369227 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go @@ -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) } @@ -84,20 +83,37 @@ 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). 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 { + // No ExecsByPath entry for this path — back-compat: treat as + // "no argv constraint", match. + 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) + } } } diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go b/pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go index 085e2215fc..625559e67c 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/exec_test.go @@ -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", @@ -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, }, } @@ -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{} diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go b/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go index 885ace3f4c..46784e7b84 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go @@ -86,7 +86,7 @@ func TestIntegrationWithAllFunctions(t *testing.T) { }, { name: "Check file access pattern", - expression: `ap.was_path_opened_with_flags(containerID, "/etc/passwd", ["O_RDONLY"])`, + expression: `ap.was_path_opened(containerID, "/etc/passwd")`, expectedResult: true, }, { diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/open.go b/pkg/rulemanager/cel/libraries/applicationprofile/open.go index ec0a8310c5..fccf19a10d 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/open.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/open.go @@ -6,7 +6,6 @@ import ( "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/cache" - "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/celparse" "github.com/kubescape/node-agent/pkg/rulemanager/profilehelper" "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" ) @@ -46,45 +45,6 @@ func (l *apLibrary) wasPathOpened(containerID, path ref.Val) ref.Val { return types.Bool(false) } -func (l *apLibrary) wasPathOpenedWithFlags(containerID, path, flags ref.Val) ref.Val { - if l.objectCache == nil { - return types.NewErr("objectCache is nil") - } - - containerIDStr, ok := containerID.Value().(string) - if !ok { - return types.MaybeNoSuchOverloadErr(containerID) - } - - pathStr, ok := path.Value().(string) - if !ok { - return types.MaybeNoSuchOverloadErr(path) - } - - // flags projection (OpenFlagsByPath) is out of scope for v1; degrade to path-only matching. - if _, err := celparse.ParseList[string](flags); err != nil { - return types.NewErr("failed to parse flags: %v", err) - } - - cp, _, err := profilehelper.GetProjectedContainerProfile(l.objectCache, containerIDStr) - if err != nil { - return cache.NewProfileNotAvailableErr("%v", err) - } - - for openPath := range cp.Opens.Values { - if dynamicpathdetector.CompareDynamic(openPath, pathStr) { - return types.Bool(true) - } - } - for _, openPath := range cp.Opens.Patterns { - if dynamicpathdetector.CompareDynamic(openPath, pathStr) { - return types.Bool(true) - } - } - - return types.Bool(false) -} - func (l *apLibrary) wasPathOpenedWithSuffix(containerID, suffix ref.Val) ref.Val { if l.objectCache == nil { return types.NewErr("objectCache is nil") @@ -105,17 +65,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. @@ -149,17 +113,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. diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/open_test.go b/pkg/rulemanager/cel/libraries/applicationprofile/open_test.go index bf407611e0..9fce787aeb 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/open_test.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/open_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types" "github.com/goradd/maps" "github.com/kubescape/node-agent/pkg/config" "github.com/kubescape/node-agent/pkg/objectcache" @@ -12,134 +13,108 @@ import ( "github.com/stretchr/testify/assert" ) -func TestOpenInProfile(t *testing.T) { - objCache := objectcachev1.RuleObjectCacheMock{ - ContainerIDToSharedData: maps.NewSafeMap[string, *objectcache.WatchedContainerData](), +// TestWasPathOpenedWithSuffix_PatternsNotScanned pins the contract from +// the CodeRabbit PR #43 review on open.go:79 (Major). Wildcard-shaped +// entries in cp.Opens.Patterns MUST NOT contribute to suffix/prefix +// answers — their literal text answers the wrong question. A retained +// pattern "/var/log/pods/*/volumes/...." doesn't END with "foo.log" +// even though the concrete open it stands in for might. Only concrete +// paths in cp.Opens.Values are valid sources of suffix/prefix truth in +// pass-through (Opens.All=true) mode. +// +// In projection-active mode (Opens.All=false), the rule manager +// precomputes Opens.SuffixHits / PrefixHits from the spec, which is +// the correct mechanism — those are exercised in +// TestOpenWithSuffixInProfile / TestOpenWithPrefixInProfile. +// +// This test exercises the pass-through path directly by setting a +// ProjectedContainerProfile where Opens.All=true, Values contains a +// concrete path with the queried suffix, and Patterns contains a +// wildcard-pattern that ALSO appears to satisfy strings.HasSuffix +// against the queried suffix. The pattern must be ignored. +func TestWasPathOpenedWithSuffix_PatternsNotScanned(t *testing.T) { + // Pass-through pcp (Opens.All=true): + // Values: ["/var/log/concrete.log"] — concrete, ends with ".log" + // Patterns: ["/var/log/⋯/foo.log"] — wildcard, ALSO ends with ".log" + // Querying suffix=".log" should match Values; we then strip + // concrete.log from Values and assert suffix doesn't match + // through Patterns alone. + pcp := &objectcache.ProjectedContainerProfile{ + Opens: objectcache.ProjectedField{ + All: true, + Values: map[string]struct{}{"/var/log/concrete.log": {}}, + Patterns: []string{"/var/log/⋯/foo.log"}, + }, + } + objCache := &mockObjectCacheForPattern{pcp: pcp} + lib := &apLibrary{objectCache: objCache} + + // 1) With concrete in Values: returns true. + got := lib.wasPathOpenedWithSuffix(types.String("test-cid"), types.String(".log")) + if b, _ := got.Value().(bool); !b { + t.Fatalf("suffix '.log' against concrete /var/log/concrete.log: expected true, got %v", got) + } + + // 2) Strip Values; only the wildcard Pattern remains. Suffix '.log' + // text-matches the pattern but the pattern is wildcardised — the + // correct answer is false (no concrete observation supports it). + pcp.Opens.Values = map[string]struct{}{} + got = lib.wasPathOpenedWithSuffix(types.String("test-cid"), types.String(".log")) + if b, _ := got.Value().(bool); b { + t.Errorf("suffix '.log' against ONLY wildcard pattern /var/log/⋯/foo.log: "+ + "expected false (patterns must not be scanned), got %v", got) } +} - objCache.SetSharedContainerData("test-container-id", &objectcache.WatchedContainerData{ - ContainerType: objectcache.Container, - ContainerInfos: map[objectcache.ContainerType][]objectcache.ContainerInfo{ - objectcache.Container: { - { - Name: "test-container", - }, - }, +// TestWasPathOpenedWithPrefix_PatternsNotScanned mirrors the suffix +// test for the prefix path. Same rabbit finding (open.go:79 Also +// applies to: 111-123). +func TestWasPathOpenedWithPrefix_PatternsNotScanned(t *testing.T) { + pcp := &objectcache.ProjectedContainerProfile{ + Opens: objectcache.ProjectedField{ + All: true, + Values: map[string]struct{}{"/var/concrete/foo": {}}, + Patterns: []string{"/var/⋯/log/foo"}, }, - }) - - profile := &v1beta1.ApplicationProfile{} - profile.Spec.Containers = append(profile.Spec.Containers, v1beta1.ApplicationProfileContainer{ - Name: "test-container", - Opens: []v1beta1.OpenCalls{ - { - Path: "/etc/passwd", - Flags: []string{"O_RDONLY"}, - }, - { - Path: "/tmp/test.txt", - Flags: []string{"O_WRONLY", "O_CREAT"}, - }, - }, - }) - objCache.SetApplicationProfile(profile) - - env, err := cel.NewEnv( - cel.Variable("containerID", cel.StringType), - cel.Variable("path", cel.StringType), - AP(&objCache, config.Config{}), - ) - if err != nil { - t.Fatalf("failed to create env: %v", err) } + objCache := &mockObjectCacheForPattern{pcp: pcp} + lib := &apLibrary{objectCache: objCache} - testCases := []struct { - name string - containerID string - path string - expectedResult bool - }{ - { - name: "Path exists in profile", - containerID: "test-container-id", - path: "/etc/passwd", - expectedResult: true, - }, - { - name: "Path does not exist in profile", - containerID: "test-container-id", - path: "/etc/nonexistent", - expectedResult: false, - }, - { - name: "Another path exists in profile", - containerID: "test-container-id", - path: "/tmp/test.txt", - expectedResult: true, - }, + got := lib.wasPathOpenedWithPrefix(types.String("test-cid"), types.String("/var/")) + if b, _ := got.Value().(bool); !b { + t.Fatalf("prefix '/var/' against concrete /var/concrete/foo: expected true, got %v", got) } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ast, issues := env.Compile(`ap.was_path_opened(containerID, path)`) - 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": tc.containerID, - "path": tc.path, - }) - if err != nil { - t.Fatalf("failed to eval program: %v", err) - } - - actualResult := result.Value().(bool) - assert.Equal(t, tc.expectedResult, actualResult, "ap.was_path_opened result should match expected value") - }) + pcp.Opens.Values = map[string]struct{}{} + got = lib.wasPathOpenedWithPrefix(types.String("test-cid"), types.String("/var/")) + if b, _ := got.Value().(bool); b { + t.Errorf("prefix '/var/' against ONLY wildcard pattern /var/⋯/log/foo: "+ + "expected false (patterns must not be scanned), got %v", got) } } -func TestOpenNoProfile(t *testing.T) { - objCache := objectcachev1.RuleObjectCacheMock{} - - env, err := cel.NewEnv( - cel.Variable("containerID", cel.StringType), - cel.Variable("path", cel.StringType), - AP(&objCache, config.Config{}), - ) - if err != nil { - t.Fatalf("failed to create env: %v", err) - } - - ast, issues := env.Compile(`ap.was_path_opened(containerID, path)`) - if issues != nil { - t.Fatalf("failed to compile expression: %v", issues.Err()) - } +// mockObjectCacheForPattern returns a fixed ProjectedContainerProfile +// for any containerID; used only by the suffix/prefix pattern tests +// above to bypass the full RuleObjectCacheMock setup. +type mockObjectCacheForPattern struct { + objectcache.ObjectCache + pcp *objectcache.ProjectedContainerProfile +} - program, err := env.Program(ast) - if err != nil { - t.Fatalf("failed to create program: %v", err) - } +func (m *mockObjectCacheForPattern) ContainerProfileCache() objectcache.ContainerProfileCache { + return &mockCPCForPattern{pcp: m.pcp} +} - result, _, err := program.Eval(map[string]interface{}{ - "containerID": "test-container-id", - "path": "/etc/passwd", - }) - if err != nil { - t.Fatalf("failed to eval program: %v", err) - } +type mockCPCForPattern struct { + objectcache.ContainerProfileCache + pcp *objectcache.ProjectedContainerProfile +} - actualResult := result.Value().(bool) - assert.False(t, actualResult, "ap.was_path_opened should return false when no profile is available") +func (m *mockCPCForPattern) GetProjectedContainerProfile(_ string) *objectcache.ProjectedContainerProfile { + return m.pcp } -func TestOpenWithFlagsInProfile(t *testing.T) { +func TestOpenInProfile(t *testing.T) { objCache := objectcachev1.RuleObjectCacheMock{ ContainerIDToSharedData: maps.NewSafeMap[string, *objectcache.WatchedContainerData](), } @@ -167,10 +142,6 @@ func TestOpenWithFlagsInProfile(t *testing.T) { Path: "/tmp/test.txt", Flags: []string{"O_WRONLY", "O_CREAT"}, }, - { - Path: "/var/log/app.log", - Flags: []string{"O_RDWR", "O_APPEND"}, - }, }, }) objCache.SetApplicationProfile(profile) @@ -178,7 +149,6 @@ func TestOpenWithFlagsInProfile(t *testing.T) { env, err := cel.NewEnv( cel.Variable("containerID", cel.StringType), cel.Variable("path", cel.StringType), - cel.Variable("flags", cel.ListType(cel.StringType)), AP(&objCache, config.Config{}), ) if err != nil { @@ -189,64 +159,31 @@ func TestOpenWithFlagsInProfile(t *testing.T) { name string containerID string path string - flags []string expectedResult bool }{ { - name: "Path and flags match exactly", - containerID: "test-container-id", - path: "/etc/passwd", - flags: []string{"O_RDONLY"}, - expectedResult: true, - }, - { - // v1 degradation: flags projection is out of scope; path-only matching. - name: "Path matches but flags don't match", + name: "Path exists in profile", containerID: "test-container-id", path: "/etc/passwd", - flags: []string{"O_WRONLY"}, expectedResult: true, }, { - name: "Path doesn't exist", + name: "Path does not exist in profile", containerID: "test-container-id", path: "/etc/nonexistent", - flags: []string{"O_RDONLY"}, expectedResult: false, }, { - name: "Multiple flags match", - containerID: "test-container-id", - path: "/tmp/test.txt", - flags: []string{"O_WRONLY", "O_CREAT"}, - expectedResult: true, - }, - { - name: "Multiple flags in different order", - containerID: "test-container-id", - path: "/tmp/test.txt", - flags: []string{"O_CREAT", "O_WRONLY"}, - expectedResult: true, - }, - { - name: "Partial flags match", + name: "Another path exists in profile", containerID: "test-container-id", path: "/tmp/test.txt", - flags: []string{"O_WRONLY"}, - expectedResult: true, - }, - { - name: "Empty flags list", - containerID: "test-container-id", - path: "/etc/passwd", - flags: []string{}, expectedResult: true, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - ast, issues := env.Compile(`ap.was_path_opened_with_flags(containerID, path, flags)`) + ast, issues := env.Compile(`ap.was_path_opened(containerID, path)`) if issues != nil { t.Fatalf("failed to compile expression: %v", issues.Err()) } @@ -259,32 +196,30 @@ func TestOpenWithFlagsInProfile(t *testing.T) { result, _, err := program.Eval(map[string]interface{}{ "containerID": tc.containerID, "path": tc.path, - "flags": tc.flags, }) if err != nil { t.Fatalf("failed to eval program: %v", err) } actualResult := result.Value().(bool) - assert.Equal(t, tc.expectedResult, actualResult, "ap.was_path_opened_with_flags result should match expected value") + assert.Equal(t, tc.expectedResult, actualResult, "ap.was_path_opened result should match expected value") }) } } -func TestOpenWithFlagsNoProfile(t *testing.T) { +func TestOpenNoProfile(t *testing.T) { objCache := objectcachev1.RuleObjectCacheMock{} env, err := cel.NewEnv( cel.Variable("containerID", cel.StringType), cel.Variable("path", cel.StringType), - cel.Variable("flags", cel.ListType(cel.StringType)), AP(&objCache, config.Config{}), ) if err != nil { t.Fatalf("failed to create env: %v", err) } - ast, issues := env.Compile(`ap.was_path_opened_with_flags(containerID, path, flags)`) + ast, issues := env.Compile(`ap.was_path_opened(containerID, path)`) if issues != nil { t.Fatalf("failed to compile expression: %v", issues.Err()) } @@ -297,40 +232,13 @@ func TestOpenWithFlagsNoProfile(t *testing.T) { result, _, err := program.Eval(map[string]interface{}{ "containerID": "test-container-id", "path": "/etc/passwd", - "flags": []string{"O_RDONLY"}, }) if err != nil { t.Fatalf("failed to eval program: %v", err) } actualResult := result.Value().(bool) - assert.False(t, actualResult, "ap.was_path_opened_with_flags should return false when no profile is available") -} - -func TestOpenWithFlagsCompilation(t *testing.T) { - objCache := objectcachev1.RuleObjectCacheMock{} - - env, err := cel.NewEnv( - cel.Variable("containerID", cel.StringType), - cel.Variable("path", cel.StringType), - cel.Variable("flags", cel.ListType(cel.StringType)), - AP(&objCache, config.Config{}), - ) - if err != nil { - t.Fatalf("failed to create env: %v", err) - } - - // Test that the function compiles correctly - ast, issues := env.Compile(`ap.was_path_opened_with_flags(containerID, path, flags)`) - if issues != nil { - t.Fatalf("failed to compile expression: %v", issues.Err()) - } - - // Test that we can create a program - _, err = env.Program(ast) - if err != nil { - t.Fatalf("failed to create program: %v", err) - } + assert.False(t, actualResult, "ap.was_path_opened should return false when no profile is available") } func TestOpenCompilation(t *testing.T) { From d0eaf0bdf3cf4fd9fa209d18a0379f01319d82bd Mon Sep 17 00:00:00 2001 From: entlein Date: Fri, 15 May 2026 22:56:36 +0200 Subject: [PATCH 2/3] restoring ap_was_path_opened_with_flags Signed-off-by: entlein --- .../cel/libraries/applicationprofile/ap.go | 22 +++++++++ .../applicationprofile/integration_test.go | 2 +- .../cel/libraries/applicationprofile/open.go | 47 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go index fabf311c2e..ce86d7ab88 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go @@ -111,6 +111,25 @@ func (l *apLibrary) Declarations() map[string][]cel.FunctionOpt { }), ), }, + "ap.was_path_opened_with_flags": { + cel.Overload( + "ap_was_path_opened_with_flags", []*cel.Type{cel.StringType, cel.StringType, cel.ListType(cel.StringType)}, cel.BoolType, + cel.FunctionBinding(func(values ...ref.Val) ref.Val { + if len(values) != 3 { + return types.NewErr("expected 3 arguments, got %d", len(values)) + } + if l.detailedMetrics && l.metrics != nil { + l.metrics.IncHelperCall("ap.was_path_opened_with_flags") + } + wrapperFunc := func(args ...ref.Val) ref.Val { + return l.wasPathOpenedWithFlags(args[0], args[1], args[2]) + } + cachedFunc := l.functionCache.WithCache(wrapperFunc, "ap.was_path_opened_with_flags", cache.HashForContainerProfile(l.objectCache)) + result := cachedFunc(values[0], values[1], values[2]) + return cache.ConvertProfileNotAvailableErrToBool(result, false) + }), + ), + }, "ap.was_path_opened_with_suffix": { cel.Overload( "ap_was_path_opened_with_suffix", []*cel.Type{cel.StringType, cel.StringType}, cel.BoolType, @@ -335,6 +354,9 @@ func (e *apCostEstimator) EstimateCallCost(function, overloadID string, target * case "ap.was_path_opened": // Cache lookup + O(n) linear search + dynamic path comparison cost = 25 + case "ap.was_path_opened_with_flags": + // Cache lookup + O(n) search + dynamic path comparison + O(f*p) flag comparison + cost = 40 case "ap.was_path_opened_with_suffix": // Cache lookup + O(n) linear search + O(n*len(suffix)) string suffix checks cost = 20 diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go b/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go index 46784e7b84..885ace3f4c 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/integration_test.go @@ -86,7 +86,7 @@ func TestIntegrationWithAllFunctions(t *testing.T) { }, { name: "Check file access pattern", - expression: `ap.was_path_opened(containerID, "/etc/passwd")`, + expression: `ap.was_path_opened_with_flags(containerID, "/etc/passwd", ["O_RDONLY"])`, expectedResult: true, }, { diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/open.go b/pkg/rulemanager/cel/libraries/applicationprofile/open.go index fccf19a10d..62a4abedfa 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/open.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/open.go @@ -6,6 +6,7 @@ import ( "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/cache" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/celparse" "github.com/kubescape/node-agent/pkg/rulemanager/profilehelper" "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" ) @@ -45,6 +46,52 @@ 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") + } + + containerIDStr, ok := containerID.Value().(string) + if !ok { + return types.MaybeNoSuchOverloadErr(containerID) + } + + pathStr, ok := path.Value().(string) + if !ok { + return types.MaybeNoSuchOverloadErr(path) + } + + // flags projection (OpenFlagsByPath) is out of scope for v1; degrade to path-only matching. + if _, err := celparse.ParseList[string](flags); err != nil { + return types.NewErr("failed to parse flags: %v", err) + } + + cp, _, err := profilehelper.GetProjectedContainerProfile(l.objectCache, containerIDStr) + if err != nil { + return cache.NewProfileNotAvailableErr("%v", err) + } + + for openPath := range cp.Opens.Values { + if dynamicpathdetector.CompareDynamic(openPath, pathStr) { + return types.Bool(true) + } + } + for _, openPath := range cp.Opens.Patterns { + if dynamicpathdetector.CompareDynamic(openPath, pathStr) { + return types.Bool(true) + } + } + + return types.Bool(false) +} + func (l *apLibrary) wasPathOpenedWithSuffix(containerID, suffix ref.Val) ref.Val { if l.objectCache == nil { return types.NewErr("objectCache is nil") From 70a9d9b644043807e2a42da05255351bb4ac661b Mon Sep 17 00:00:00 2001 From: entlein Date: Sat, 16 May 2026 13:19:07 +0200 Subject: [PATCH 3/3] apply rabbit feedback: align R0040 args consumer with rc1 final state Signed-off-by: entlein --- .../cel/libraries/applicationprofile/exec.go | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go index 5f57369227..10e8d4c978 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go @@ -92,14 +92,40 @@ func (l *apLibrary) wasExecutedWithArgs(containerID, path, args ref.Val) ref.Val // 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 { if profileArgs, ok := cp.ExecsByPath[pathStr]; ok { if dynamicpathdetector.CompareExecArgs(profileArgs, runtimeArgs) { return types.Bool(true) } } else { - // No ExecsByPath entry for this path — back-compat: treat as - // "no argv constraint", match. + // State 2: ExecsByPath absent → back-compat "no argv constraint". return types.Bool(true) } }