Ensure other group members can read/edit audit log#91
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the local backend’s filesystem ownership/permission handling so that when a shared group is configured, the audit log (and related storage paths) become group-writable by default, addressing issue #80.
Changes:
- Add Unix-only helpers/constants to enforce specific permission modes (directories, blobs, audit log).
- Apply group + mode enforcement during
init,store, andlog_auditwhen a group is configured. - Add Unix-only tests to verify permission behavior with and without a configured group.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if let Some(group_name) = &self.group { | ||
| log::debug!( | ||
| "Setting group {} on {}", | ||
| group_name, | ||
| path.as_ref().display() | ||
| ); | ||
| let path = path.as_ref(); | ||
| let gid = resolve_group(group_name)?; | ||
| chown(path.as_ref(), None, Some(gid))?; | ||
| if fs::metadata(path)?.gid() == gid.as_raw() { |
There was a problem hiding this comment.
Pull request overview
Updates the LocalBackend (local filesystem storage) to ensure that when a Unix group is configured, storage directories and the audit log are created/normalized with group-shared permissions so other group members can read/edit the audit log (per #80).
Changes:
- Add helpers/constants to apply configured group ownership and enforce specific Unix permission modes.
- Set shared modes on storage directories, blob files, and the audit log when
groupis configured (Unix). - Add Unix-only tests validating permission behavior with and without a configured group.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| use nix::unistd::{Group, getegid}; | ||
| use std::os::unix::fs::PermissionsExt; | ||
|
|
||
| let current_group_name = Group::from_gid(getegid()).unwrap().unwrap().name; |
| } | ||
|
|
||
| // On non-Unix, a configured group is intentionally a no-op. We still need | ||
| // blobs to become read-only eg on Windows, so this cannot be just `self.group.is_some()`. |
dpastoor
left a comment
dpastoor
left a comment
There was a problem hiding this comment.
how will these changes also play out with the desire to allow the specificity of either not allowing others to see at all, or allowing everyone to read and/or write. I lost track of we talked about dropping expressing things as the octal permissions and instead allowing people to express them as settings values in the config, but forget if we implemented that/what the options were. Did we do that/does that map to this PR?
the "default" scenario definitely seems implemnted
Closes #80