saltstack / saltstack/salt

file.managed omits mode from changes when creating a file (test=True and real runs), making created-file permissions unauditable

Open
#69,969 1 comment 1 reaction 0 assignees View on GitHub

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:

  1. 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.

  2. 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_sls exposes 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.

  3. 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 changes
    plan, 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 both test=True and the real run.
  • So drift detection and idempotency on existing files are unaffected. Only the initial creation omits the mode.
  • show_changes: False is not a factor; behaviour is identical with and without it.
  • The unspecified-mode case 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.