Skip to content

fix(security): create the sync log and profile directories owner-only (0700) - #104

Draft
dash0-dev[bot] wants to merge 1 commit into
mainfrom
security/log-and-profile-dirs-owner-only
Draft

dash0-dev[bot] wants to merge 1 commit into
mainfrom
security/log-and-profile-dirs-owner-only

Conversation

@dash0-dev

@dash0-dev dash0-dev Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Summary

The per-profile sync logs (~/.local/log/synctray-sync-{shortId}.log) and the profile JSON (~/.config/synctray/profiles/) live in directories SyncTray creates with the default mode, 0755. On a Mac where other accounts can traverse the user's home folder, another local user can list and read them: the name of every synced file, remote names, local paths, and anything typed into additionalRcloneFlags. This change creates both directories 0700 and tightens them on existing installs once per launch.

Finding

  • Severity: low. The attacker is another local user on the same Mac. Precondition: the victim's home folder is traversable by other accounts. macOS home folders grant group staff read+execute by default, so ~/Public works, and every standard user is in staff. File contents and rclone.conf are not exposed; rclone.conf is already 0600.
  • Class: CWE-276 Incorrect Default Permissions
  • Location:
    • /// Profile-specific log file
      var logPath: String {
      "\(NSHomeDirectory())/.local/log/synctray-sync-\(shortId).log"
      }
    • // Create ~/.local/log if needed
      let logDir = (profile.logPath as NSString).deletingLastPathComponent
      if !fm.fileExists(atPath: logDir) {
      try fm.createDirectory(atPath: logDir, withIntermediateDirectories: true)
      }
      // Create ~/.config/synctray/profiles if needed
      if !fm.fileExists(atPath: SyncProfile.configDirectory) {
      try fm.createDirectory(
      atPath: SyncProfile.configDirectory, withIntermediateDirectories: true)
      }
    • // Create log file and directory if they don't exist
      let fileManager = FileManager.default
      if !fileManager.fileExists(atPath: logPath) {
      let directory = (logPath as NSString).deletingLastPathComponent
      try? fileManager.createDirectory(atPath: directory, withIntermediateDirectories: true)
      fileManager.createFile(atPath: logPath, contents: nil)
      }
    • private func writeProfileFiles() {
      guard (try? FileManager.default.createDirectory(
      atPath: profilesDirectory, withIntermediateDirectories: true)) != nil else { return }
    • let fm = FileManager.default
      guard (try? fm.createDirectory(
      atPath: directory, withIntermediateDirectories: true)) != nil else { return nil }
    • static func writeProfileFiles(from profileDicts: [[String: Any]], to directory: String) throws -> WriteResult {
      try FileManager.default.createDirectory(atPath: directory, withIntermediateDirectories: true)
    • // Ensure log directory exists (append to existing log, don't truncate)
      let logDir = (syncLogPath as NSString).deletingLastPathComponent
      try? fileManager.createDirectory(atPath: logDir, withIntermediateDirectories: true)
      (same pattern at L4020-L4022 and L4428-L4431)
  • Shipped in latest release (v0.110.0): yes. The code is also unchanged in v0.111.0, released after the audited commit.

Evidence

  1. The log path sits in a shared, conventional directory (SyncProfile.swift#L172-L175):
    var logPath: String {
        "\(NSHomeDirectory())/.local/log/synctray-sync-\(shortId).log"
    }
  2. Install creates that directory and the profiles directory with no attributes, so both get the default 0755 (SyncSetupService.swift#L2545-L2555):
    let logDir = (profile.logPath as NSString).deletingLastPathComponent
    if !fm.fileExists(atPath: logDir) {
        try fm.createDirectory(atPath: logDir, withIntermediateDirectories: true)
    }
    if !fm.fileExists(atPath: SyncProfile.configDirectory) {
        try fm.createDirectory(
            atPath: SyncProfile.configDirectory, withIntermediateDirectories: true)
    }
    The same no-attributes creation happens in LogWatcher.startWatching (L41-L47), ProfileStore (L125-L127, L158-L160), the v3 migration (MigrationRunner.swift#L320-L321) and three manual-run paths in ProfileDetailView. The log file itself is created with the default 0644, both by createFile and by the launchd agent's StandardOutPath.
  3. What ends up in the log: the script runs rclone with --verbose (SyncSetupService.swift#L2210) and appends all of its output to the log (L2309, "${RCLONE_CMD[@]}" 2>&1 | tee -a "$LOG_FILE"). That records one line for every transferred or deleted path, plus lines such as Using primary remote: $REMOTE_NAME (L1474). The profile JSON holds remote names, local paths and additionalRcloneFlags.

No scanner rule; found by manual review.

Reproduction

  1. On a Mac with two standard accounts, A and B, have account A create a profile and let it sync once.
  2. As A: stat -f '%Sp %N' ~/.local/log ~/.local/log/synctray-sync-*.log ~/.config/synctray/profiles prints drwxr-xr-x, -rw-r--r--, drwxr-xr-x.
  3. As B: ls -ld /Users/A shows r-x for group staff, and head /Users/A/.local/log/synctray-sync-*.log prints A's synced file names.

With this change, step 2 shows drwx------ for both directories, and step 3 fails with "Permission denied".

Fix

  • New directories are owner-only. Every place that creates ~/.local/log or ~/.config/synctray/profiles now passes attributes: [.posixPermissions: 0o700]: SyncSetupService.createDirectories, LogWatcher, both ProfileStore writers, the v3 migration, and the three ProfileDetailView log paths. A 0700 directory protects the files inside regardless of their own mode, so the log files and profile JSON need no per-file change. Foundation also applies these attributes to any intermediate directory it creates, such as a missing ~/.local.
  • Existing installs are fixed at launch. SyncSetupService.tightenPrivateDirectoriesIfLoose() runs once per launch from SyncTrayApp, next to the existing tightenConfigPermissionsIfLoose(), and uses the same approach. For each of the two directories it checks four things: the path exists, it is a real directory (attributesOfItem does not follow a final symlink), it is owned by getuid(), and it has group or other bits set. If all four hold, it sets the mode to mode & 0o700. It never creates anything and never recurses.
  • A new SyncProfile.logDirectory is now the single source of the log directory; logPath builds on it.
  • The generated script and the launchd plists are deliberately unchanged. A global umask there would also change the permissions rclone gives the user's synced files.
  • Regression test: ConfigSelfTest AC-SEC5 (log-and-profile-dirs-owner-only) checks five cases. A 0755 directory is tightened to 0700. An already-0700 directory is left unchanged. A symlink and its 0755 target are left alone. A regular file and a missing path are left alone, and nothing is created. ProfileStore.writeProfileFile creates a new directory 0700. A source pin also asserts the launch-time call is present.
  • CLAUDE.md's Generated Files table documents the invariant.

Verification

  • Ran in the audit sandbox (Linux):
    • git diff --check: clean.
    • An rg over SyncTray/ for createDirectory on the log and profile directories: all 9 sites pass 0o700, and none remain without attributes.
    • Swift could not be built or run here (no Swift toolchain), so neither compilation nor AC-SEC5 was executed.
  • Needs macOS CI: the build, and SyncTray.app/Contents/MacOS/SyncTray --self-test printing AC-SEC5 log-and-profile-dirs-owner-only: PASS with every other AC still passing. A manual check on a real install would confirm that an upgraded ~/.local/log and ~/.config/synctray/profiles read drwx------ after one launch.

Opened by the weekly security audit automation. It stays a draft until a maintainer reviews it.

… (0700)

The sync logs name every synced file and the profile JSON names remotes, local paths and extra rclone flags, but ~/.local/log and ~/.config/synctray/profiles were created with the default mode (0755). On a Mac whose home folder other accounts can traverse, another local user could read them (CWE-276).

Create both directories 0700 at every creation site, and tighten an existing install once per launch (a real directory owned by this user, group/other bits dropped, never followed through a symlink, nothing created), mirroring tightenConfigPermissionsIfLoose. Covered by ConfigSelfTest AC-SEC5.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@dash0-dev dash0-dev Bot added the security-audit Opened by the weekly security audit automation label Oct 12, 2026
@dash0-dev

dash0-dev Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

✅ No issues found

Creates ~/.local/log and the profiles directory 0700 at all nine creation sites and tightens existing installs at launch. Correct as written; one non-blocking test-coverage suggestion.

1 advisory finding below the confidence bar — see Less certain below.

Checked: 9 of 9 changed files read · 2 possible issues → 0 confirmed → 0 posted

Less certain (1) — advisory, below the confidence bar
  • SyncTray/Services/ConfigSelfTest.swift:3747 — suggestion: Only writeProfileFile is asserted, so dropping the 0700 attribute in createDirectories or LogWatcher stays green and leaves the logs readable until the next launch. (confidence 89)
Review details
Gate Status Details
Description vs. code ✅ Matches the diff: 9 creation sites, launch-time tighten, AC-SEC5, CLAUDE.md row.
Prior review feedback ✅ Earlier review comments are resolved.
Documentation ✅ Body covers threat, repro, fix, verification; doc comments and CLAUDE.md record the invariant.
Self-review signals ✅ No debug logs, leftover TODOs, or unreviewed stubs.
Code review ✅ The multi-lens review found no blocking issues.

Found

Quality — produced 2 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 1
Dropped — 1 below-bar
Optimality — ran (in-context) - approach mirrors tightenConfigPermissionsIfLoose; a SyncTray-owned log subdir would cost a log-path migration; 0 proposals
Standards — ran - 2 docs, 149 bullets; no violations (AC-SEC5 registered in the run list, CLAUDE.md Generated Files row updated)
Measurability — ran (in-context) - quiet: no new user-facing or request-serving path; silent chmod failure matches sibling tightenConfigPermissionsIfLoose

Run

full · 139 lines in delta · tier deep · depth checkout
⚠️ 2 prepare-time anomalies (reviewer identity unknown (no --reviewer-login / PR_REVIEWER_LOGIN; /user 401s here) — relation defaulted to cross, +1 more)
Integrations — not activated (no manifest or lockfile in the diff)
Memories — 61 indexed · 2 used (2 lessons)

  • lesson codebase-knowledge::hotspot::SyncTray/Services/ConfigSelfTest.swift — checklist shape (2): guard on one site of a symmetric set leaves the twins unpinned
  • lesson codebase-knowledge::hotspot::SyncTray/Services/SyncSetupService.swift — checklist (7): a mode set at creation covers only new dirs; confirmed the launch-time tighten exists

Nothing to report — severity, 0 files skipped.

pr-reviewer · commit 069aee3 · full review · how these findings are produced · updated Oct 12, 2026 5:31am UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-audit Opened by the weekly security audit automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants