file.managed omits mode from changes when creating a file (test=True and real runs), making created-file permissions unauditable
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
When file.managed creates a file, the mode it applies is never included in changes — in neither the test=True plan nor the real run's output. When the file already exists and only its mode is wrong, the mode is reported. Same module, same property, observability depends on whether the file happens to already exist.
The mode itself is applied correctly, so this is not a vulnerability — it is a reporting/auditability defect. But it has a consequence worth taking seriously:
A state declaring mode: '0600' and an otherwise identical state declaring no mode at all produce byte-identical output, in both the plan and the real run, while producing different results on disk (0600 vs a umask-derived mode, 0644 under a typical root umask). Neither Salt's pre-application plan nor its post-application output distinguishes a root-only file from a world-readable one.
Three things make this more than cosmetic:
-
Absence is misleading, not merely missing. Because the existing-file path does print a
mode:line, an operator can reasonably read the absence of that line as "no mode is being managed here" — which is exactly inverted. -
The run output is the audit record. Job returns are what get retained (job cache, external returners, log shipping). That retained record permanently omits the mode of every file Salt creates, so there is no in-band retrospective verification either.
state.show_slsexposes the declared mode, but that is intent rather than applied effect, it is not the retained record, and under conditional Jinja it need not match what a given run actually did. -
The gap is irrecoverable, because creation is a one-shot event. Over the entire lifetime of a correctly-declared
mode: '0600'file, the mode appears in no output at any point:stage reported changesplan, before creation {'newfile': '/tmp/lifetime.txt'}real apply (creation) {'diff': 'New file'}plan, after creation {}— "is in the correct state"real apply, again {}— "is in the correct state"Re-running cannot back-fill the record: subsequent runs assert conformance without disclosing the property being conformed to. The only moment the mode could be captured is the moment it is omitted, and that moment never recurs.
This matters most in a CI / change-control workflow, where test=True is run on a proposed change, the plan is reviewed and stored as the authorising artifact, and idempotency is asserted from that baseline forward. For every file the run creates, the permissions are absent from that baseline and unrecoverable downstream — so an auditor reviewing the plan for a state that writes secrets must assume the worst case, with no way to discharge that assumption from the artifact then or later.
Setup
Masterless local run (salt-call --local). Nothing environment-specific — reproduced with inline contents:, no fileserver or template involved.
Steps to Reproduce
Two states differing only in the presence of mode:
# with_mode.sls
/tmp/secretfile.txt:
file.managed:
- contents: 'mysql_root_password=hunter2'
- mode: '0600'
# without_mode.sls
/tmp/secretfile.txt:
file.managed:
- contents: 'mysql_root_password=hunter2'
rm -f /tmp/secretfile.txt
salt-call --local --out=json state.apply with_mode test=True
rm -f /tmp/secretfile.txt
salt-call --local --out=json state.apply without_mode test=True
changes for both plans, identical:
{ "newfile": "/tmp/secretfile.txt" }
Applying each for real:
with mode 0600 -> actual mode: 600 reported changes: {'diff': 'New file'}
with NO mode -> actual mode: 644 reported changes: {'diff': 'New file'}
For contrast, with the file already existing, content already correct, and only the mode wrong (0644 -> 0600):
Changes:
mode:
0600
Notes / scope — the defect is specific to the creation path; reporting on existing files is complete:
- Existing file, wrong mode and wrong content: reports both,
{'diff': ..., 'mode': '0600'}— a content change does not mask the mode. replace: False+ wrong mode: reports{'mode': '0600'}in bothtest=Trueand the real run.- So drift detection and idempotency on existing files are unaffected. Only the initial creation omits the mode.
show_changes: Falseis not a factor; behaviour is identical with and without it.- The unspecified-
modecase is umask-derived, not a fixed default (umask 022 -> 644, 000 -> 666, 077 -> 600), so what an unspecified-mode state produces is environment-dependent. - A declared mode is applied reliably regardless of umask (
umask 000+mode: '0600'-> 600).
Expected Behavior
When file.managed creates a file, changes should include the mode (and ideally user/group) being applied, e.g.:
Changes:
newfile:
/tmp/secretfile.txt
mode:
0600
— so the plan is sufficient to review the permissions of a to-be-created file, and the run output is sufficient to verify them after the fact, consistent with the existing-file path.
Actual Behavior
changes contains only newfile (test=True) or diff: New file (real run). The mode is omitted, making a correct secrets-file state and an accidentally world-readable one indistinguishable in both the plan and the retained result.
Versions Report
salt-call 3008.2 (Argon)
Minion OS: Ubuntu 24.04 (osfinger: Ubuntu-24.04), saltversion: 3008.2.
Additional context
Found while reviewing the test=True plan of a state that writes an installer answer-file containing a plaintext database root password. The plan reported only newfile:, which gave no way to confirm from the plan that the file would land 0600 rather than umask-default — the single most important property to verify about that state before letting it run.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the Python implementation and existing tests for the file.managed creation path, comparing it with the existing-file mode-reporting path. Reproduce the issue with the two inline states and test=True, then add regression coverage showing that a newly created file reports its declared mode in both planned and real changes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- devops, infrastructure
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 65/100