fix(utils): restore NormalizePath to fix headless /proc and dot paths#774
fix(utils): restore NormalizePath to fix headless /proc and dot paths#774yugal07 wants to merge 2 commits intokubescape:mainfrom
Conversation
Signed-off-by: yugal07 <yashsadhwani544@gmail.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/utils/normalize_path_test.go (1)
28-63: Add boundary cases for terminal headless proc pathsPlease add cases for
/46/fdand/46/taskso regex boundary behavior is locked down.Suggested test additions
{ name: "headless proc path (fd)", input: "/46/fd/3", expected: "/proc/46/fd/3", }, + { + name: "headless proc path (fd terminal)", + input: "/46/fd", + expected: "/proc/46/fd", + }, + { + name: "headless proc path (task terminal)", + input: "/46/task", + expected: "/proc/46/task", + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/normalize_path_test.go` around lines 28 - 63, Add two boundary test cases to the existing test table in normalize_path_test.go to exercise terminal headless proc paths: one with input "/46/fd" expecting "/proc/46/fd" and one with input "/46/task" expecting "/proc/46/task"; place them alongside the other cases (use descriptive names like "terminal headless proc path (fd)" and "terminal headless proc path (task)") so the normalization logic exercised by the test runner (the test table used by the test function in this file) validates regex boundary behavior for those endings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/utils/path.go`:
- Around line 9-10: The regex in headlessProcRegex currently only matches when
the matched segment has a trailing slash, so terminal paths like "/1234/task" or
"/1234/fd" are skipped; update the pattern used by headlessProcRegex (and the
other similar regexes referenced around lines 25-27) to accept either a trailing
slash or end-of-string (for example: allow (task|fd) followed by either "/" or
end), then run tests to ensure both "/pid/task", "/pid/task/", "/pid/fd" and
"/pid/fd/" are all recognized as headless.
---
Nitpick comments:
In `@pkg/utils/normalize_path_test.go`:
- Around line 28-63: Add two boundary test cases to the existing test table in
normalize_path_test.go to exercise terminal headless proc paths: one with input
"/46/fd" expecting "/proc/46/fd" and one with input "/46/task" expecting
"/proc/46/task"; place them alongside the other cases (use descriptive names
like "terminal headless proc path (fd)" and "terminal headless proc path
(task)") so the normalization logic exercised by the test runner (the test table
used by the test function in this file) validates regex boundary behavior for
those endings.
🪄 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: da44170b-5c4c-4437-812c-47b05fea3fb0
📒 Files selected for processing (4)
pkg/utils/datasource_event.gopkg/utils/normalize_path_test.gopkg/utils/path.gopkg/utils/struct_event.go
Signed-off-by: yugal07 <yashsadhwani544@gmail.com>
1 participant
Overview
Commit 9597da4 refactored datasource_event.go to suppress verbose field
length warnings, but silently dropped the NormalizePath() calls that were
introduced in eafe066. This caused raw eBPF paths to be surfaced without
normalization, resulting in:
This PR restores pkg/utils/path.go with the NormalizePath() function and
re-wires it into the GetPath, GetFullPath, GetExePath, GetNewPath, and
GetOldPath getter methods in both DatasourceEvent and StructEvent.
Testing -
Deploy the node-agent and inspect open events or an ApplicationProfile.
Paths should be fully absolute (e.g. /proc/46/task/46/fd) with no bare .
entries.
Unit tests can be run locally with:
go test ./pkg/utils/...
This fixes #721
Summary by CodeRabbit
Bug Fixes
Tests