Read real Windows ACLs in ai_tools fsutil.Stat - #50772
Merged
juan-fdz-hawa merged 2 commits intoAug 10, 2026
Merged
Conversation
juan-fdz-hawa
temporarily deployed
to
Docker Hub
August 7, 2026 14:50 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe change adds cross-platform permission detection for Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@orbit/pkg/table/ai_tools/internal/fsutil/perm_acl.go`:
- Around line 67-72: Update the ACL evaluation logic around the write decision
handling to track FILE_WRITE_DATA, FILE_APPEND_DATA, DELETE, WRITE_DAC, and
WRITE_OWNER independently rather than collapsing them through writeMask. Set
WorldWritable when any of these effective capabilities is allowed, while
preserving per-capability deny precedence; add a regression case covering DELETE
denied followed by FILE_WRITE_DATA allowed.
In `@orbit/pkg/table/ai_tools/internal/fsutil/perm_unix.go`:
- Around line 11-19: Update the permission lookup around os.Lstat in the
relevant Unix permission function to reject non-regular files, including
symlinks, by returning Perm{Known: false} before evaluating mode bits. Only
compute WorldReadable and WorldWritable for regular files, matching the
OpenRegular behavior on Windows.
In `@orbit/pkg/table/ai_tools/internal/fsutil/perm_windows.go`:
- Around line 42-45: In the DACL evaluation logic, make incomplete or
unsupported evaluation return an unknown posture: at the AceCount limit check,
return Perm{} when truncation would occur instead of evaluating a shortened ACL,
and in the non-inherit-only unsupported ACE branch return Perm{} rather than
skipping it. Add tests covering both cases and verify Known is false.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 538a6a5f-9195-44dd-ae22-247c041bda8f
📒 Files selected for processing (7)
.github/workflows/test-go-windows.ymlorbit/pkg/table/ai_tools/internal/fsutil/fsutil.goorbit/pkg/table/ai_tools/internal/fsutil/perm_acl.goorbit/pkg/table/ai_tools/internal/fsutil/perm_acl_test.goorbit/pkg/table/ai_tools/internal/fsutil/perm_unix.goorbit/pkg/table/ai_tools/internal/fsutil/perm_windows.goorbit/pkg/table/ai_tools/internal/fsutil/perm_windows_test.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #50772 +/- ##
=======================================
Coverage 68.53% 68.53%
=======================================
Files 3977 3979 +2
Lines 256148 256181 +33
Branches 13627 13627
=======================================
+ Hits 175561 175586 +25
- Misses 64973 64978 +5
- Partials 15614 15617 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Resolves #50703 Stat() returned Perm{Known: false} unconditionally on Windows, so the world_writable risk flag on agent_instruction rows could never fire no matter what the file's real ACL was. Split the permission read per platform: perm_unix.go keeps the mode-bit logic, perm_windows.go reads the DACL via GetSecurityInfo, and perm_acl.go holds the platform-neutral ACE decision rules so they are unit-testable anywhere. Known: false now means "could not determine" rather than "Windows". Everyone (S-1-1-0) and Authenticated Users (S-1-5-11) count as world; BUILTIN\Users is excluded because standard locations grant it create rights by default. Also adds ./orbit/pkg/table/ai_tools/... to the Windows CI package list, which otherwise never exercises this code.
juan-fdz-hawa
force-pushed
the
50703-ai_tools-world_writable-risk-flag-never-fires-on-windows
branch
from
August 10, 2026 14:00
01e55fb to
8ae6498
Compare
juan-fdz-hawa
temporarily deployed
to
Docker Hub
August 10, 2026 14:00 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
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 `@orbit/pkg/table/ai_tools/internal/fsutil/perm_acl.go`:
- Around line 43-49: Update worldPermFromACEs to expand GENERIC_READ,
GENERIC_WRITE, and GENERIC_ALL in every ACE mask using the Windows file
generic-rights mapping before applying DACL precedence and computing fresh.
Ensure generic denies affect corresponding specific read/write rights, while
preserving the existing writeMask and readMask evaluations.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ff3d9ef6-5e50-4991-9850-beee5d746d0c
📒 Files selected for processing (10)
.github/workflows/test-go-windows.ymlorbit/pkg/table/ai_tools/internal/fsutil/fsutil.goorbit/pkg/table/ai_tools/internal/fsutil/perm_acl.goorbit/pkg/table/ai_tools/internal/fsutil/perm_acl_test.goorbit/pkg/table/ai_tools/internal/fsutil/perm_acl_windows.goorbit/pkg/table/ai_tools/internal/fsutil/perm_unix.goorbit/pkg/table/ai_tools/internal/fsutil/perm_windows.goorbit/pkg/table/ai_tools/internal/fsutil/perm_windows_test.goorbit/pkg/table/ai_tools/internal/instructions/instructions_test.goorbit/pkg/table/ai_tools/internal/mcp/risk_test.go
juan-fdz-hawa
temporarily deployed
to
Docker Hub
August 10, 2026 14:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
juan-fdz-hawa
marked this pull request as ready for review
August 10, 2026 14:54
juan-fdz-hawa
requested review from
a team,
georgekarrv,
getvictor,
lucasmrod,
lukeheath and
sharon-fdm
as code owners
August 10, 2026 14:54
dantecatalfamo
approved these changes
Aug 10, 2026
getvictor
approved these changes
Aug 10, 2026
getvictor
left a comment
getvictor
left a comment
Member
There was a problem hiding this comment.
Approving .github/workflows/test-go-windows.yml change
juan-fdz-hawa
deleted the
50703-ai_tools-world_writable-risk-flag-never-fires-on-windows
branch
August 10, 2026 21:30
This was referenced Aug 11, 2026
juan-fdz-hawa
added a commit
that referenced
this pull request
Aug 11, 2026
This was referenced Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #50703
Stat() returned Perm{Known: false} unconditionally on Windows, so the world_writable risk flag on agent_instruction rows could never fire no matter what the file's real ACL was.
Split the permission read per platform: perm_unix.go keeps the mode-bit logic, perm_windows.go reads the DACL via GetSecurityInfo, and perm_acl.go holds the platform-neutral ACE decision rules so they are unit-testable anywhere. Known: false now means "could not determine" rather than "Windows".
Everyone (S-1-1-0) and Authenticated Users (S-1-5-11) count as world; BUILTIN\Users is excluded because standard locations grant it create rights by default.
Also adds ./orbit/pkg/table/ai_tools/... to the Windows CI package list, which otherwise never exercises this code.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Testing
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes