Skip to content

Add cross-platform secure file permissions with Windows DACL support#73

Merged
wesm merged 6 commits intowesm:mainfrom
hughdbrown:chore-securewrite-securemkdir
Feb 5, 2026
Merged

Add cross-platform secure file permissions with Windows DACL support#73
wesm merged 6 commits intowesm:mainfrom
hughdbrown:chore-securewrite-securemkdir

Conversation

@hughdbrown
Copy link
Contributor

Problem

Unix permission modes (0600, 0700) used throughout the codebase are no-ops on Windows for access control. On a multi-user Windows machine, OAuth tokens, emails, and attachments are readable by other users.

Solution

Create internal/fileutil/ package with build-tagged implementations:

  • Unix: thin wrappers around os.WriteFile, os.MkdirAll, os.Chmod, os.OpenFile
  • Windows: after calling the standard os function, set a DACL restricting access to the current user when the mode is owner-only (perm & 0077 == 0)

internal/fileutil/

  • secure_unix.go (//go:build !windows) — thin wrappers around os.WriteFile, os.MkdirAll, os.Chmod, os.OpenFile
  • secure_windows.go (//go:build windows) — calls the standard os function, then for owner-only modes (perm & 0077 == 0) applies a DACL via golang.org/x/sys/windows that grants GENERIC_ALL only to the current user with PROTECTED_DACL_SECURITY_INFORMATION to block inherited ACEs. SID lookup failures log a warning and fall back to default behavior.
  • secure_test.go — 6 tests covering all 4 functions with owner-only and permissive modes, error paths, and read-only open

Usage

File Internal calls
internal/oauth/oauth.go │ os.MkdirAll → SecureMkdirAll (0700), os.Chmod → SecureChmod (0600)
internal/config/config.go 2x os.MkdirAll → SecureMkdirAll (0700)
internal/deletion/manifest.go os.WriteFile → SecureWriteFile (0600)
internal/sync/sync.go os.WriteFile → SecureWriteFile (0600)
cmd/msgvault/cmd/export_eml.go os.WriteFile → SecureWriteFile (0600)
cmd/msgvault/cmd/export_attachment.go os.OpenFile → SecureOpenFile (0600)
internal/export/attachments.go 2x os.OpenFile → SecureOpenFile (perm param)
internal/update/update.go os.WriteFile → SecureWriteFile (0600)

hughdbrown and others added 2 commits February 5, 2026 08:43
On Windows, Unix permission modes (0600, 0700) are no-ops for access
control. This introduces internal/fileutil with build-tagged
implementations: thin wrappers on Unix, and DACL-setting variants on
Windows that restrict owner-only files to the current user's SID.

Updates 10 call sites across 8 files where sensitive data (OAuth tokens,
email content, deletion manifests, attachments) is written with
owner-only permissions.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- restrictToCurrentUser now returns actual errors instead of swallowing
  them, making the function signature honest and letting callers decide
- All callers (SecureWriteFile, SecureMkdirAll, SecureChmod, SecureOpenFile)
  log DACL failures as warnings but don't fail the operation
- SecureMkdirAll now secures all intermediate directories created by
  os.MkdirAll, not just the leaf directory
- Document the TOCTOU window in SecureOpenFile on Windows

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@wesm wesm changed the title Chore securewrite securemkdir Add cross-platform secure file permissions with Windows DACL support Feb 5, 2026
wesm and others added 3 commits February 5, 2026 10:37
…ky tests

- Add CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE to directory DACLs so
  child files and subdirectories automatically inherit owner-only access
- Secure os.MkdirTemp result in config.go with SecureChmod(0700)
- Replace exact permission assertions with umask-tolerant helper that
  checks no extra bits are set beyond the requested mode

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
…n tests

- Apply SecureChmod(0700) to all MkTempDir return paths (preferred, system,
  and fallback), not just the fallback. Previously preferred and system temp
  dirs could inherit permissive ACLs on Windows.
- Extract secureTempDir helper that logs warnings on failure instead of
  silently discarding errors.
- Fix SecureOpenFile comment: DACL is intentionally applied whenever
  O_CREATE is set, not only for newly created files — all callers write
  sensitive data that should be owner-only.
- Add assertTempDirSecured helper and permission checks to MkTempDir tests.
- Make fallback test permission assertion umask-tolerant.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@hughdbrown
Copy link
Contributor Author

hughdbrown commented Feb 5, 2026

This is weird:

Run go build -tags fts5 -o msgvault.exe ./cmd/msgvault
# github.com/wesm/msgvault/internal/fileutil
Error: internal\fileutil\secure_windows.go:47:23: cannot use inherit (variable of type int) as uint32 value in struct literal
Error: Process completed with exit code 1.

Claude told me the Windows cross-compilation was planned:

Implementation Order
1. Create internal/fileutil/secure_unix.go 
2. Create internal/fileutil/secure_windows.go
3. Create internal/fileutil/secure_test.go 
4. Update 8 source files (add import, swap function calls)
5. go mod tidy (promotes golang.org/x/sys to direct dep)  
6. make test and make lint 
7. Cross-compile check: GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build ./internal/fileutil/                                       ```

and worked:

⏺ All tests pass. Now lint and cross-compile check.
...
⏺ All clean. go vet passes and the Windows cross-compile succeeds.

@wesm
Copy link
Owner

wesm commented Feb 5, 2026

That is a little weird. just fixing the windows build issue, stand by

The untyped constants NO_INHERITANCE, CONTAINER_INHERIT_ACE, and
OBJECT_INHERIT_ACE default to int when assigned to a variable, but
EXPLICIT_ACCESS.Inheritance is uint32. Use explicit type declaration.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@wesm wesm merged commit 3c14ede into wesm:main Feb 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants