Skip to content

fix: add unix domain socket path length validation#6150

Closed
mukeshmahato17 wants to merge 3 commits intoenvoyproxy:mainfrom
mukeshmahato17:udslength
Closed

fix: add unix domain socket path length validation#6150
mukeshmahato17 wants to merge 3 commits intoenvoyproxy:mainfrom
mukeshmahato17:udslength

Conversation

@mukeshmahato17
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This PR adds validation to limit Unix domain socket path length to 108 characters, which is the standard limit across Unix systems. This limitation exists for historical reasons related to the kernel's mbuf (memory buffer) data structure in early BSD implementations.

Which issue(s) this PR fixes:

Fixes #6145

Release Notes: Yes

@mukeshmahato17 mukeshmahato17 requested a review from a team as a code owner May 22, 2025 04:39
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// +kubebuilder:validation:XValidation:rule="self.size() <= 108",message="Unix domain socket path length must not exceed 108 characters"

Do we need this validation after +kubebuilder:validation:MaxLength=108?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hm, yes we don't need redundant validation that is unnecessary

@mukeshmahato17 mukeshmahato17 force-pushed the udslength branch 2 times, most recently from e44ded6 to ef4dcd7 Compare May 22, 2025 08:16
@zhaohuabing
Copy link
Copy Markdown
Member

@mukeshmahato17 run make gen-check locally and submit the changes will fix the gen-check.

@codecov
Copy link
Copy Markdown

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.54%. Comparing base (36651e7) to head (ef4dcd7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6150      +/-   ##
==========================================
+ Coverage   70.52%   70.54%   +0.02%     
==========================================
  Files         219      219              
  Lines       36342    36342              
==========================================
+ Hits        25629    25638       +9     
+ Misses       9192     9184       -8     
+ Partials     1521     1520       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: mukeshmahato17 <mukeshmahato1089@gmail.com>
Signed-off-by: mukeshmahato17 <mukeshmahato1089@gmail.com>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we have a new test instead of replacing an existing one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually this was merge conflict. i resolved the conflict only

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.

Limit UDS Path length to 108 with CEL

3 participants