[16.0-stable] pillar: fix decimal literals used as file modes

[christoph-zededa]

Description

Backport of #6264

Six file-mode arguments in pillar are written as decimal literals, so the
permission bits they produce are not the ones they read as:

Literal	Go value	Resulting permission bits
755	0o1363	0o363 — -wxrw--wx
644	0o1204	0o204 — -w----r--

Measured, not inferred:

os.MkdirAll(dir, 755)  -> d-wxrw--wx (0363)
os.MkdirAll(dir, 0755) -> drwxr-xr-x (0755)

So the vault directories were created world-writable, and the attestation
integrity token file (/run/eve.integrity_token) was written world-readable
while not being readable by its owner. Affected call sites:

• pkg/pillar/vault/key.go — key staging directory
• pkg/pillar/vault/handler_ext4.go (x2) — default vault and vault path
• pkg/pillar/vault/handler_unsupported.go — default vault
• pkg/pillar/vault/handler_zfs.go — sealed dataset mount point
• pkg/pillar/cmd/zedagent/attesttask.go — integrity token file

Each site is switched to an octal literal keeping the permissions it already
intended, so there is no change of intent anywhere — only the notation bug.

Cherry-picked with git cherry-pick -x, applies cleanly with no conflicts.

Only the fix commit of #6264 is backported. That PR's second commit,
semgrep: run the rules from make and in CI, is developer tooling (a make
target and a CI workflow) rather than a fix, and it modifies
tests/semgrep-rules/os-openfile-non-perm-mode.yaml, which is absent from
16.0-stable, 14.5-stable and 13.4-stable.

In addition, this PR carries one fix that is not part of #6264, because the
affected code does not exist on master and so cannot be cherry-picked:

pkg/pillar/dpcreconciler/linuxitems/wlan.go creates /run/wlan with
os.Mkdir(RunWlanDir, 600). 600 decimal is 0o1130, permission bits 0o130
(--x-wx---), so the directory is group-writable and not readable by its
owner — and it holds the generated wpa_supplicant configuration, i.e. the WiFi
credentials. It is changed to 0700 rather than 0600, since a directory also
needs the execute bit to be traversable. On master this code was removed by
b92d1ff, which moved wpa_supplicant into pillar.

How to test and validate this PR

The mistake is checkable statically. This reports nothing after this PR, and
the six call sites above before it:

grep -rnE '\.(MkdirAll|Mkdir|WriteFile|OpenFile|Chmod)\(([^()]|\([^()]*\))*,\s*(os\.FileMode\()?[1-9][0-9]*\s*\)?\)' \
    --include='*.go' . | grep -v /vendor/

Also run:

make -C pkg/pillar fmt-check
make -C pkg/pillar vet
make -C pkg/pillar test

On a device, the permission change can be confirmed directly. Since MkdirAll
only applies its mode when creating the directory, the vault checks need a
device whose vault does not exist yet (fresh install, or an installation
without TPM where the vault is a plain folder):

1. Boot a device with this build and confirm the vault directory is
   drwxr-xr-x rather than d-wxrw--wx:
   stat -c '%a %A %n' /persist/vault
2. Confirm the vault still unlocks normally and app instances with encrypted
   volumes start. All the affected directories are used by root-owned
   processes only, so no functional change is expected.
3. On a ZFS install, confirm the sealed dataset mount point is drwxr-xr-x:
   stat -c '%a %A %n' /persist/vault
4. After a successful attestation, confirm the integrity token file is
   -rw-r--r-- rather than --w----r--:
   stat -c '%a %A %n' /run/eve.integrity_token
5. Confirm the wlan run directory is drwx------ rather than d--x-wx---
   on a device with a WiFi port configured:
   stat -c '%a %A %n' /run/wlan

Suggested QA regression focus: vault creation and unlock on a fresh install,
both with and without TPM, on ext4 and zfs; plus one attestation cycle.

Changelog notes

Fixed file permissions on the vault directories, the vault key staging
directory and the attestation integrity token file, which were created with
different permission bits than intended.

PR Backports

Original: #6264

• 17.0-stable: [17.0-stable] pillar: fix decimal literals used as file modes #6281
• 16.0-stable: [16.0-stable] pillar: fix decimal literals used as file modes #6282 (this PR)
• 14.5-stable: [14.5-stable] pillar: fix decimal literals used as file modes #6283
• 13.4-stable: [13.4-stable] pillar: fix decimal literals used as file modes #6284

Checklist

• I've provided a proper description
• I've added the proper documentation
• I've tested my PR on amd64 device
• I've tested my PR on arm64 device
• I've written the test verification instructions
• I've set the proper labels to this PR

For backport PRs (remove it if it's not a backport):

• I've added a reference link to the original PR
• PR's title follows the template

And the last but not least:

• I've checked the boxes above, or I've provided a good reason why I didn't
  check them

Reasons for the unchecked boxes:

• Documentation: no user-facing or architectural behaviour changes, so there is
  nothing to document.
• Device testing on amd64/arm64: not yet done, hence the draft status. The
  changes are architecture-independent, but the vault paths are device
  management code, so the on-device steps above still need to be run before
  this leaves draft.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 23.55%. Comparing base (9f69150) to head (d263448).
⚠️ Report is 144 commits behind head on 16.0-stable.

Additional details and impacted files

@@               Coverage Diff               @@
##           16.0-stable    #6282      +/-   ##
===============================================
+ Coverage        19.52%   23.55%   +4.02%     
===============================================
  Files               19       19              
  Lines             3021     2509     -512     
===============================================
+ Hits               590      591       +1     
+ Misses            2310     1787     -523     
- Partials           121      131      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vivek-zededa]

Verification — PASS, with three follow-ups

Verified this backport independently (system test). The change does what it says; every
number in the description reproduces exactly. Three follow-ups below, none of which block
this PR — the first is inherited from master, the other two are separate gaps this PR
happens to expose.

What was verified

Backport fidelity — the fix commit is content-identical to 12049361 on master
(only hunk offsets differ, as expected). wlan.go is correctly carried as an extra
commit rather than a cherry-pick, since b92d1ff35 removed that code on master.

The mode arithmetic, measured on linux/arm64 with umask 0 rather than reasoned about:

literal	Go value	Perm()	as created
755	0o1363	0363	d-wxrw--wx
644	0o1204	0204	--w----r--
600	0o1130	0130	d--x-wx---

Matches the description exactly. (Go ignores mode bits 1<<9…1<<18; only Perm()
plus setuid/setgid/sticky reach the syscall, which is why 0o1363 lands as 0363.)

Completeness, using the two mode rules from tests/semgrep-rules/ on master:

before (this PR's base) : 8 findings
after  (this PR's head) : 1 finding

All 7 non-octal-file-mode hits are the sites fixed here — a clean sweep for that rule.
The surviving finding is item 2 below.

gofmt -l clean on all six files; go vet ./dpcreconciler/linuxitems/ clean and the
dpcreconciler tests pass.

Also confirming the 0700-not-0600 choice for /run/wlan is right: installWifiConfig
creates the config inside that directory and Chmod(0600)s it, so the directory needs
the traverse bit and the file is already owner-only.

---

1. The key.go fix is masked by the tmpfs mounted over it

stageKey() now creates the directory 0700, then two lines later mounts a tmpfs on top
of it with no -o mode=:

if err := os.MkdirAll(keyDirName, 0700); err != nil { ... }

if _, _, err := execCmd("mount", "-t", "tmpfs", "tmpfs", keyDirName); err != nil { ... }

A tmpfs root defaults to 01777, so the mode underneath is masked for as long as the
mount lives — which is exactly the window in which the unsealed vault key is staged there.
Measured:

after mkdir 0700              : 700  drwx------
mount -t tmpfs tmpfs DIR      : 1777 drwxrwxrwt     <- masks it
mount -t tmpfs -o mode=0700   : 700  drwx------     <- would hold

The commit message for 9b8801f3 already identifies this ("that is not sufficient on its
own … which masks the mode underneath for as long as the unsealed vault key is staged
there"), but the diff does not add the mount option, so the stated conclusion isn't
carried into the code.

Impact is limited — the key file itself is written 0700, so the key bytes are not
exposed; what remains is a world-writable staging directory (sticky, so entries can't be
removed by others) at /run/TmpVaultDir2, /TmpVaultDir1 and keyDir.

This is inherited from master, not introduced here — origin/master:vault/key.go has
the identical mount call. So it belongs upstream rather than in this backport; flagging
it here only because this is where it was found. Fix would be
"mount", "-t", "tmpfs", "-o", "mode=0700", "tmpfs", keyDirName.

Unrelated nit in the same function: os.WriteFile(keyFileName, vaultKey, 0700) gives a
key file the execute bit; 0600 looks like the intent.

2. agentlog.go still has a mode of this family on this branch

The one semgrep finding that survives this PR:

// pkg/pillar/agentlog/agentlog.go, printToFile()
f, err := os.OpenFile(filename, os.O_APPEND|os.O_WRONLY|os.O_CREATE, os.ModeAppend)

os.ModeAppend is 1<<30; its permission bits are 0000, so the file is created mode
0000. Master uses 0644 here.

This is a missing backport, not a new bug: 319babc38 "fix(agentlog): use proper file
permissions when appending logs" is on origin/16.5 … origin/16.14 and origin/17.0,
but not on 16.0-stable, 14.5-stable or 13.4-stable. Same three branches as this PR's
siblings, so it may be worth folding into the same backport sweep.

3. The stable branches have no guard against this returning

tests/semgrep-rules/ exists on 16.0-stable, but only with the bigint-bytes rules —
non-octal-file-mode.yaml and os-openfile-non-perm-mode.yaml are master-only. The
description explains why #6264's tooling commit was excluded, which is reasonable, but the
result is that the branches which just had this bug are the ones with no static check for
it. Since the rule files are self-contained and the semgrep-rules directory already
exists here, carrying just those two YAMLs across would close it without pulling in the
make/CI changes.

---

On the unchecked device-testing boxes

Worth noting for whoever runs those steps: pkg/pillar/vault/ has no test files at all,
and attesttask_test.go does not cover storeIntegrityToken. So make test passing says
nothing about these six call sites — the stat checks in the description are the only real
verification, and they still need a device with a vault that doesn't exist yet
(MkdirAll only applies its mode on creation, as the description correctly notes).

_{Verified by Vivek (System Test) with Claude Code — static checks and runtime mode
measurements only, no device run.}