Skip to content

Harden local credential file permissions#5

Open
SunZhiC wants to merge 1 commit intoLoongphy:mainfrom
SunZhiC:main
Open

Harden local credential file permissions#5
SunZhiC wants to merge 1 commit intoLoongphy:mainfrom
SunZhiC:main

Conversation

@SunZhiC
Copy link
Copy Markdown

@SunZhiC SunZhiC commented Mar 11, 2026

Summary

  • enforce 0600 for sensitive auth/registry files written by codex-auth on Unix-like systems
  • enforce 0700 for ~/.codex/accounts on Unix-like systems
  • keep Windows behavior as warn-only (no hard failure for POSIX mode enforcement)
  • add registry permission tests and update implementation docs

Validation

  • zig build test
  • zig build run -- list

@Loongphy
Copy link
Copy Markdown
Owner

Loongphy commented Mar 11, 2026

Thanks for your PR. There are some issues with the Codex review comment:

  • It points to a real problem, but the more immediate blocking issue is a
    runtime regression in the new directory-permission hardening path. On this
    branch, zig test src/main.zig -lc reliably crashes when
    ensureAccountsDir() calls hardenDirectoryPermissions(), with the panic
    coming from dir.chmod(...) inside Zig stdlib (std.posix.fchmod via
    std.fs.Dir.chmod).
  • I would avoid the blanket claim that codex-auth list crashes in
    general. I did not reproduce that consistently: zig build run -- list
    succeeded for me, including with a fresh temporary CODEX_HOME. The test
    crash is still enough to make this a blocking regression.
  • Because of that runtime regression, the PR should not be merged as-is,
    even though the hardening goal itself is reasonable.
  • I did not find an obvious malicious or data-exfiltration risk in the PR.
    The concern is correctness and runtime stability, not intent.

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