Skip to content

Conversation

@macladson
Copy link
Contributor

Issue Addressed

#5

Proposed Changes

Adds a file_mode method to the builder to set file permissions in octal format. For example to set restrictive permissions:

.file_mode(0o600)

This also sets compressed files to these permissions as well.

This PR also contains a small architectural change (using self more liberally) which made a few things easier for me.

Considerations

Note that technically any u32 value will be accepted, even if it would break file permissions. For example .file_mode(600) is not equivalent to `.file_mode(0o600). It is highly recommended to use octal format otherwise you might get unexpected file permissions. There are a few validation techniques we could use, but none of them felt particularly clean. One option we could consider is an enum like this:

enum FileMode {
    Permissive, // Equivalent to 0o666
    Default, // Equivalent to 0o644
    Restrictive, // Equivalent to 0o600
    ...
    Custom(u32), // Users set their own mode.
}

Then for the non-Custom variants we could ensure the mode is set to octal, which then would only expose the footgun to users who opt in to a custom mode.

However this adds some bloat, and I appreciate the logroller is very lightweight, so we might just deem this footgun acceptable.

@trayvonpan trayvonpan merged commit e4a8bb5 into alasdairpan:main Apr 1, 2025
@macladson macladson deleted the add-file-mode branch April 2, 2025 11:07
mergify bot pushed a commit to sigp/lighthouse that referenced this pull request May 22, 2025
Update our `logroller` dependency to the new version which supports permission control. See -> alasdairpan/logroller#6
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.

2 participants