Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for umask on isMorePermissive #23109

Closed
wants to merge 3 commits into from

Conversation

lucasmrod
Copy link
Member

@lucasmrod lucasmrod commented Oct 22, 2024

Tentative fix for #22877, to support different values of umask.

Still waiting for user feedback, thus opening as draft.

Alternative, just document fleetctl package is currently expected to work with umask = 002 (my Ubuntu VMs) or umask = 022 (macOS / most Linux distros).

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Added/updated tests
  • Manual QA for all new/changed functionality
  • For Orbit and Fleet Desktop changes:
    • Orbit runs on macOS, Linux and Windows. Check if the orbit feature/bugfix should only apply to one platform (runtime.GOOS).
    • Manual QA must be performed in the three main OSs, macOS, Windows and Linux.
    • Auto-update manual QA, from released version of component to new version (see tools/tuf/test).

@lucasmrod lucasmrod changed the title Account for umask on isMorePermissive Account for umask on isMorePermissive Oct 23, 2024
@iansltx
Copy link
Member

iansltx commented Oct 23, 2024

Given that this involvse finicky file system stuff, probably best to add tests for both getUmask and isMorePermissive, in part to document expected behavior. I can kind of follow what's going on here but there are enough layers between the existing tests and the changed methods that something closer to unit tests would be helpful here.

@lucasmrod
Copy link
Member Author

Given that this involvse finicky file system stuff, probably best to add tests for both getUmask and isMorePermissive, in part to document expected behavior. I can kind of follow what's going on here but there are enough layers between the existing tests and the changed methods that something closer to unit tests would be helpful here.

I think I'll move towards documenting that currently supported umask values for fleetctl package are 002/022 (default for most Linux distros and macOS).

What was the correct place to document this kind of thing?

@iansltx
Copy link
Member

iansltx commented Oct 23, 2024

Maybe here: https://fleetdm.com/guides/enroll-hosts#cli, or elsewhere on that page

@lucasmrod
Copy link
Member Author

Closing in favor of #23120 (documenting requirement for now, to reduce complexity/risk).

@lucasmrod lucasmrod closed this Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants