fix(cli): create the connector key file atomically with owner-only permissions

[Chris0Jeky]

Summary

The CLI's connector encryption key file is now created atomically with owner-only permissions
instead of being written first and restricted afterwards.

CliFirstRunBootstrapper.PersistKey stages the generated key in a sibling temp file and then
File.Moves it into place. It used to call File.WriteAllText(tempPath, payload) and only then
File.SetUnixFileMode(tempPath, ...), and only on Unix. This change routes the staged write through
a new CLI-local helper, backend/src/Taskdeck.Cli/RestrictedFileWriter.cs, which creates the file
with FileMode.CreateNew and FileShare.None, supplies the owner-only permissions to the create
call itself, verifies them through the still-open handle, and writes the payload through that same
handle. The existing MoveWithRetry into place is unchanged.

RestrictedFileWriter is a deliberate, faithful copy of the API-side helper
Taskdeck.Api.FirstRun.FirstRunBootstrapper.WriteRestrictedFile / CreateRestrictedNewFile /
CreateOwnerOnlyFileWindows / BuildOwnerOnlyFileSecurity, shipped by PR #1267 for #1264. The CLI
cannot reference the API project (Taskdeck.Cli references Application and Infrastructure only, and
Taskdeck.Architecture.Tests enforces that), so the code is duplicated rather than shared. The class
comment names the API original and PR #1267, and the API helper itself is not moved or edited.

Root cause

PersistKey created the temp file with File.WriteAllText, which applies whatever permissions the
platform defaults give it, and only afterwards narrowed them.

On Unix that leaves a window between the write and the SetUnixFileMode call in which the base64
256-bit connector key sits on disk at the umask-derived mode. Under the common umask 022 that is
0644, readable by every local user. File.Move preserves the mode, so the exposure is the window
rather than the final file.

On Windows nothing narrowed the permissions at all. The SetUnixFileMode call was inside an
if (!OperatingSystem.IsWindows()) guard, so the temp file simply inherited the containing
directory's DACL, which typically grants BUILTIN\Users read. File.Move preserves what was
inherited, so the persisted key file stayed readable by other local users for its whole life, not
just for a window.

The fix removes both problems by never letting the file exist with wider permissions than intended:
on Unix the file is born 0600 via FileStreamOptions.UnixCreateMode and the exact mode is then
pinned through the open handle with File.SetUnixFileMode(SafeFileHandle), which is umask-proof and
fails on filesystems that cannot store the mode; on Windows a protected owner-only FileSecurity
(inheritance disabled, a single ACE for the current user's SID) is passed to FileInfo.Create and
the resulting DACL is read back through the same handle, so FAT32, exFAT and SMB shares that
silently ignore the security descriptor fail closed rather than persist the key unprotected.
FileMode.CreateNew also refuses to adopt a file someone pre-created at the temp path, and any
failure is normalized to IOException so the existing
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) in EnsureKeyOnDisk
still degrades to a transient in-memory key instead of crashing the CLI.

Verification

All commands run in the worktree at .worktrees/codex-1262-cli-key-atomic-create, Windows 11.

Red first, against the unmodified CliFirstRunBootstrapper.cs at base 86ca907, with only the new
test file added:

dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1 --filter "FullyQualifiedName~CliRestrictedFileWriterTests"

Failed!  - Failed: 2, Passed: 0, Skipped: 0, Total: 2

CliRestrictedFileWriterTests.PersistKey_WritesTheKeyViaAtomicRestrictedCreate_StructuralCheck [FAIL]
  Assert.Contains() Failure: Sub-string not found
  Not found: "RestrictedFileWriter.WriteRestrictedFile("...

CliRestrictedFileWriterTests.EnsureKeyOnDisk_LockdownSurvivesAtomicMove [FAIL]
  inheritance should be disabled so the directory's default ACEs (e.g. BUILTIN\Users read) do not apply
  at CliRestrictedFileWriterTests.AssertOwnerOnly(String path)

The second failure is the Windows half of the bug observed end to end: the key file that
EnsureConnectorEncryptionKey persists had an inherited, unprotected DACL.

Green after the fix, same filter, with the three helper-level tests added:

Passed!  - Failed: 0, Passed: 5, Skipped: 0, Total: 5

Whole CLI test project:

dotnet test backend/tests/Taskdeck.Cli.Tests/Taskdeck.Cli.Tests.csproj -c Release -m:1

Passed!  - Failed: 0, Passed: 228, Skipped: 0, Total: 228, Duration: 3 m 49 s

Layer purity, since the fix adds a file to the Cli project:

dotnet test backend/tests/Taskdeck.Architecture.Tests/Taskdeck.Architecture.Tests.csproj -c Release -m:1

Passed!  - Failed: 0, Passed: 28, Skipped: 1, Total: 29

Full solution build:

dotnet build backend/Taskdeck.sln -c Release

Build succeeded. 0 Error(s), 12 Warning(s)

All 12 warnings are pre-existing CS8602/CS8603/CS1998 in Taskdeck.Api.Tests and
Taskdeck.Application.Tests files this PR does not touch. No warning comes from
Taskdeck.Cli or Taskdeck.Cli.Tests sources changed here.

Required backend check from backend/AGENTS.md:

dotnet test backend/Taskdeck.sln -c Release -m:1

Exit code 0, all six projects green:

Passed!  - Failed: 0, Passed: 1603, Skipped:  0, Total: 1603, Duration: 609 ms   - Taskdeck.Domain.Tests.dll
Passed!  - Failed: 0, Passed: 4177, Skipped:  0, Total: 4177, Duration: 36 s     - Taskdeck.Application.Tests.dll
Passed!  - Failed: 0, Passed: 2843, Skipped:  4, Total: 2847, Duration: 5 m 34 s - Taskdeck.Api.Tests.dll
Passed!  - Failed: 0, Passed:  228, Skipped:  0, Total:  228, Duration: 3 m 19 s - Taskdeck.Cli.Tests.dll
Passed!  - Failed: 0, Passed:   28, Skipped:  1, Total:   29, Duration: 530 ms   - Taskdeck.Architecture.Tests.dll
Passed!  - Failed: 0, Passed:    7, Skipped: 29, Total:   36, Duration: 1 s      - Taskdeck.Integration.Tests.dll

Total 8886 passed, 0 failed, 34 skipped. The skips are the suites' own pre-existing conditional
skips, not anything this PR disabled.

Parity with the API original. Extracting both helper bodies and stripping comments and blank lines,
diff reports only two differences: the API's additional RestrictFileToCurrentUser method, which
is its forward-remediation path for files that already exist and has no caller in the CLI, and the
closing method boundary. The four copied methods (WriteRestrictedFile x2, CreateRestrictedNewFile,
CreateOwnerOnlyFileWindows, BuildOwnerOnlyFileSecurity) are character-identical.

Whitespace:

git diff --check 86ca9078c92d8b7da4fb98d53dcbd0b88c2fe4a1...HEAD — clean, exit 0.

node scripts/check-docs-governance.mjs was not run because no documentation file changed. I read
docs/security/BETA_THREAT_MODEL.md, docs/platform/CONFIGURATION_REFERENCE.md and
docs/decisions/ADR-0041-desktop-connector-key-autogeneration.md looking for a sentence that
describes the CLI key file's protection as write-then-chmod or as inheritance-based. There is none.
ADR-0041 documents the API and desktop-exe half and already states the file is created owner-only
before the secret is written, which stays true; CONFIGURATION_REFERENCE.md:197 describes the API's
local-config copies. Nothing became false, so no doc sentence was edited.

Not verified

• The Unix branch was not executed. This box is Windows, so CreateOwnerOnlyFileWindows and the
  Windows DACL assertions are what actually ran; the FileStreamOptions.UnixCreateMode path and the
  0600 assertions are covered only by the structural test here and by the Linux CI runner. The Unix
  code is a character-level copy of the API helper that has been running on the Linux runner since
  PR fix(security): create secret files atomically with owner-only permissions (#1264) #1267.
• The fail-closed behaviour on FAT32, exFAT and SMB shares was not exercised on a real such volume.
  It is pinned structurally (AreAccessRulesProtected, File.SetUnixFileMode(stream.SafeFileHandle)
  rather than behaviourally, the same way the API suite pins it.
• No concurrency or multi-process test was added for the temp-path collision that
  FileMode.CreateNew now refuses; the temp name already carries a fresh GUID per call.
• Frontend, integration and E2E suites were not run; nothing outside backend/src/Taskdeck.Cli
  changed.

Risk notes

• The helper is a duplicate of the API's, not a shared abstraction. Moving it into a shared layer
  would have meant touching the API helper, whose FirstRunBootstrapperTests assert its source text
  structurally, so the duplication is deliberate. It carries a comment naming the original and
  PR fix(security): create secret files atomically with owner-only permissions (#1264) #1267; a future change to the lockdown contract has to be made in both places, and the two
  structural tests will not catch a drift between them.
• The write path now throws IOException in cases that previously succeeded: a filesystem that
  cannot store the owner-only permissions, and a pre-existing file at the temp path. Both surface as
  the existing "Could not persist connector encryption key" stderr warning and a transient in-memory
  key for that run, so the CLI still starts. On such a volume the key stops being persisted at all,
  which is the intended fail-closed trade and matches the API's behaviour since FirstRunBootstrapper: create secret temp file atomically with owner-only ACL (close empty-file handle race) #1264.
• EnsureKeyOnDisk's existing catch clause covers IOException and UnauthorizedAccessException;
  the helper normalizes everything else to IOException, so no new exception type escapes to
  startup.
• The change is confined to how the temp file is created. Key generation, the cross-process mutex,
  the read-modify-write merge of appsettings.local.json, and MoveWithRetry are untouched.

Closes #1262. Refs #1241, #1264, PR #1267.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Review disposition (beta-platform-integrity lane, T3 gate)

Fresh-context adversarial review at 21697ef: SHIP. No CRITICAL or HIGH. The reviewer compared both helper sources line by line, confirmed the catch (IOException or UnauthorizedAccessException) fallback in EnsureKeyOnDisk still holds, and noted the change is strictly better on the failure paths too: before, a failed SetUnixFileMode(tempPath) left a 0644 temp file containing the key on disk permanently, because only MoveWithRetry was inside the try; now the file is deleted before any payload is written. Two candidate HIGHs were attempted and refuted (the Windows branch does run in CI on the [ubuntu-latest, windows-latest] Cli.Tests matrix; the exception-surface inventory's line references land exactly on the PR head).

Tracked, not fixed here (law 2: MEDIUM and LOW become issues)

All four are in #2667:

• MEDIUM, no forward remediation: key files created before this PR keep their inherited Windows DACL because EnsureKeyOnDisk returns as soon as ReadExisting finds a key. The API shipped RestrictExistingLocalConfigFile for this; the CLI needs the same. Out of CliFirstRunBootstrapper: owner-only Windows ACL + fix write-before-chmod TOCTOU on the connector key file #1262's acceptance criteria and a new code path, so it gets its own reviewed PR.
• MEDIUM, no drift guard between the API and CLI copies of the helper: the CLI structural test pins substrings in the CLI copy only. A parity test that diffs the shared method bodies is the follow-up.
• MEDIUM, new fail-closed behaviour on non-ACL filesystems: a data directory on FAT32/exFAT/SMB now gets a per-run transient key with a stderr warning instead of an unprotected persisted key. Same posture as the API, correct, but undocumented; one paragraph in the configuration reference is the follow-up.
• LOW, docs/COURSE_CORRECTION.md still lists CliFirstRunBootstrapper: owner-only Windows ACL + fix write-before-chmod TOCTOU on the connector key file #1262 as an accepted-risk write-off while the 2026-08-23 realignment re-lists it as pending; this PR follows the later record. Maintainer wording for the reconciliation note.

Verified by the coordinator

• LOW, base ancestry: git merge-base --is-ancestor 86ca9078c origin/main is true; the base is plain origin/main at worktree creation, not a stacked branch. The ResolveDataDirectory visibility change the reviewer saw came from the already-merged CLI failure-sink work (PR feat(cli): bounded always-on diagnostic sink for unexpected failures #2573).

Verification recorded in the PR body

Red first on the unmodified bootstrapper (structural check and the Windows DACL assertion both failed), then 5/5 focused, Cli.Tests 228/228, Architecture.Tests 28/28 (1 pre-existing skip), solution build 0 errors, the full serialized backend solution 8886 passed / 0 failed / 34 pre-existing skips, git diff --check clean. Not executed on this box: the Unix UnixCreateMode and handle-pinned 0600 branch (character-identical to the API helper that runs on the Linux runner since PR #1267; it first executes on the Ubuntu leg of this PR's CI), and fail-closed on a real non-ACL volume (pinned structurally, as the API suite does). Merge once exact-head ci-required is green and the three-minute floor has passed.