Repository navigation
Conversation
The BuildStream CAS backend was not storing or restoring critical file metadata, including file permission bits (especially setuid/setgid bits), when serializing/deserializing directory trees or exporting to tar. Key changes: - Add a 'mode' field to _IndexEntry to store full unix file mode (0o7777) - Add a 'mode' field to the public FileStat API - Capture full file mode (including setuid/setgid/sticky bits) when adding files to the CAS directory tree (via os.lstat + stat.S_IMODE) - Serialize/unserialize full mode using the protobuf unix_mode field in NodeProperties (which is already part of the Remote Execution API) - Restore full mode on checkout (both files and directories), falling back to the legacy is_executable behavior for old artifacts - Use stored mode in export_to_tar() instead of hardcoded values - In FileBasedDirectory, read full mode from os.stat_result and restore it when importing from CAS-based directories (previously it would unconditionally chmod to 0o755 for executable files, stripping setuid/ setgid) - FileBasedDirectory.export_to_tar() now uses gettarinfo() which preserves full mode (UID/GID are still normalized to 0 for reproducibility) This ensures that bootable images and tooling relying on setuid/setgid binaries are correctly produced.
LeonxLJX
requested review from
BenjaminSchubert,
abderrahim,
cs-shadow,
gtristan and
juergbi
as code owners
October 28, 2026 09:15
Contributor
|
Thank you for your contribution. However, from a brief look it doesn't seem this has been tested in any real scenario as it doesn't capture mode bits of build outputs (and there are also no tests in this PR). Furthermore, this would require configurability for security and backwards compatibility reasons. This is at least partially discussed in #38. I.e., as my impression is that this was never tested and it's far from a proper solution to #38, I'm closing this. If you want to seriously work on fixing #38, the solution should address the aspects mentioned in the issue and tests should demonstrate that it actually works. Possibly with a design proposal before a PR. |
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
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.
Fixes #38.
Problem
BuildStream's CAS backend was not storing or restoring critical file metadata including full UNIX file mode (especially setuid/setgid bits) and directory mode bits. When artifacts were serialized to CAS and checked out, setuid/setgid bits were lost, breaking bootable images and setuid/setgid tooling.
Solution
Use the Remote Execution API's existing NodeProperties.unix_mode protobuf field to preserve full file mode throughout the storage pipeline:
Files Changed
All changes are backwards-compatible with existing artifacts.