fix(execd): apply owner/group to auto-created parent directories#1075
Open
Pangjiping wants to merge 3 commits into
Open
fix(execd): apply owner/group to auto-created parent directories#1075Pangjiping wants to merge 3 commits into
Pangjiping wants to merge 3 commits into
Conversation
When uploading files or creating directories with nested paths, parent directories created by os.MkdirAll inherited the execd process ownership (typically root) instead of the requested owner/group. This caused Permission denied errors when non-root workload users attempted cleanup. Add MkdirAllWithOwnership helper that identifies newly created directories and applies SetFileOwnership only to them, leaving pre-existing directories untouched. Update resolveUploadTarget and MakeDir to use the new helper. Closes #1064 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…build Mirror the unix MkdirAllWithOwnership helper and MakeDir changes in utils_windows.go to fix the Windows cross-compilation failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bf16f4c8f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ership When targetDir itself already exists (e.g. uploading /file.txt where targetDir is /), the walk-up loop exited without clearing firstNew, causing SetFileOwnership to run on the pre-existing root directory. Rewrite the loop to start firstNew as empty and only set it when a non-existent directory is found, so pre-existing directories are never touched. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
MkdirAllWithOwnershiphelper that creates directories and appliesowner/grouponly to newly created ones, leaving pre-existing directories untouchedresolveUploadTarget(file upload path) to propagateowner/groupmetadata to auto-created parent directoriesMakeDir(create directories API) to use the same helper, fixing intermediate directory ownership for multi-level creationCloses #1064
Test plan
TestMkdirAllWithOwnership_NewNestedDirs— multi-level dirs created correctlyTestMkdirAllWithOwnership_PreExistingParent— pre-existing parent permissions unchangedTestMkdirAllWithOwnership_AllExist— all dirs exist, no-opTestMakeDir_PreExistingDir— existing regression test still passesTestMakeDir_NewDir— existing regression test still passesgo test ./pkg/web/controller/passes🤖 Generated with Claude Code