[rabbitson87]

Refs #224 (item 1). Part of #237. Items 2 to 4 of #224 stay open.

What this adds

host.secret_key_path puts the host key, secret.key, in another place, for example a Docker secret or a file that only the service user can
read. The setting is empty by default, and empty keeps the key at <data>/secret.key as before. The design is the spec
secret-key-path.md, which is marked implemented.

With the setting, the host only reads the key. It never creates, writes, renames or deletes the file and makes no temporary file beside it, so a
key on a read-only mount works. The start refuses, with status 78 and before any listener opens, when the file is missing (also when flats.db
holds no sealed value), is not a regular file of 32 bytes, cannot be read, or when a second key exists at <data>/secret.key or the key resolves
into the data directory. The message names the path and the setting.

How it works:

• The setting is an absolute path outside host.data_dir. config set and unset change it offline. config set host.data_dir refuses a
  directory that contains the key path. schema_version stays 5, and an older release refuses the new key as an unknown key.
• One type, core.KeyFile, is the location of the key. The host, core.New, the operator-key seal and open, flats key set,
  flats secret backup-key, restore-key and reset, and flats backup all get the path from it. flats key ls and rm touch no key.
• restore-key --out FILE writes the recovered key to a new file of mode 0600, for a key that only the service process can see. It changes
  nothing at the configured path.
• flats backup leaves the key out of the copy, records secret_key: "external" (format stays 1), prints the key path, reads the key twice
  and fails when it changed, checks the sealed values of the copy against the key, and scans every file of the copy for the 32 key bytes, so a
  hard link or a bind mount that escapes the path checks still fails the backup. --verify reports a note. --exclude-secret-key then means
  that the command does not read the key.
• A SIGHUP reload lists the setting as awaiting a restart.
• Docs: docs/configuration.md (the settings row, a new section on the key lifecycle, moving the key out and back, the downgrade order,
  what a refusal means, systemd), docs/container.md (Docker and Compose), docs/design.md, docs/surfaces.md, the spec index, and the
  rules of host-backup.md, secret-key-recovery.md and config-reload.md that change.

Where the change is

• internal/config/: the key host.secret_key_path, its checks, InsideDir.
• internal/core/keyfile.go and the callers in internal/core: KeyFile, CheckSecretKey, SealHostKey, OpenHostKey, reload.go.
• internal/app/: serve.go, keys.go, keycmd.go, secretkeycmd.go, backupcmd.go, netbird.go, setup.go.
• internal/backup/: the external key in backup.go, copy.go (hashScan), verify.go, manifest.go.
• Tests: internal/config/secretkeypath_test.go, internal/core/keyfile_test.go, internal/app/secretkeypath_test.go,
  internal/app/secretkeypath_cmd_test.go, internal/backup/externalkey_test.go.
• No schema change, no new dependency, go.mod is unchanged. core.ReadSecretKey and core.RestoreSecretKey are removed; nothing else used them.

Known limits

The spec states most of them.

• Hard links, bind mounts and Windows short names can give the key a second name that the path checks do not unify. The backup scan finds
  the key in a copy under any name; the start does not check.
• A key that only the service process sees (a systemd LoadCredential= file, a Docker secret) is missing for the key commands and
  flats backup, which run as processes of their own. flats backup then needs --exclude-secret-key, and restore-key needs --out.
• The docs for a systemd user unit (what flats install makes) take the credentials path from systemd.exec(5) and were not run on a real
  systemd. The Docker and Compose steps were checked by reading, not run in a container.
• A key rotation during a run has no defined effect (item 2 of feat(secrets): key outside the data directory, rotation and injected fetch secrets #224). A mid-run replacement of the file changes what operator-key actions
  open at once and does not change flat secrets.
• The tests of links, read-only folders and modes skip on Windows without the privilege or as the superuser. They ran on Linux as uid 1000.
• Local checks ran on Windows, and on Linux as uid 1000. -race ran on Linux for the packages of this change on the commit that the
  implementation review accepted, not on the last five commits (docs, wording, tests and the one line below). The CI run of the last commit passed all 25 checks.

Task report

This is the task report that the development protocol asks for. The reviewer's full reports are kept by the author on a local disk. That copy
is not accessible to other reviewers, so the lines below are exact excerpts of the verdicts and of each finding's headline. Nothing here
replaces a verdict.

• Scope: issue feat(secrets): key outside the data directory, rotation and injected fetch secrets #224 item 1, step 2 of the plan, stages design, implementation and testing. Base: origin/main at b09f5fd.
• Material author of the spec, the code and the tests: claude-sonnet-5-5.
• Reviewer: requested claude-fable-5-1; it did the review work in every run. The reviewer authored no part of the change.
• Budget: at most 2 rounds per stage. Used: design 2, implementation 2, testing 2, plus one short impact assessment after the code scanning
  failure below, which is one run beyond the plan of 2 rounds per stage. Reviewer runs: 7. Every run after the first resumed the reviewer's session.

Verdicts and findings of each review round

• Design, round 1: DESIGN: REJECT

  • D1, blocker. Rules 16, 2, 18. A leftover key file in the data directory, or a linked path, puts a copy of the key in the backup.
  • D2, major. Rule 5. "Before it writes anything" is false for what prepare writes before the check.
  • D3, major. The commands. A missing configuration file is a refusal, not "the key is <data>/secret.key".
  • D4, major. Rules 16 to 18 and 21. A systemd credential is visible only to the service process.
  • D5, major. Rule 8 and the reload. restartKeys is an explicit list.
  • D6, major. Rules 5 and 6. A wrong-size key exits with status 1 today.
  • D7, minor. Spec convention: "Current state" section.
  • D8, minor. Criteria that need modes the superuser and Windows do not have.
  • D9, minor. Rule 6. A link to nothing must not offer restore-key.
  • D10, minor. Rule 22. The order of the downgrade steps.
  • D11, minor. Rule 12. The temporary file lies beside the configured path.
  • D12, minor. Rule 2. config set cannot repair a file that the check refuses.
  • D13, minor. Rule 17. The step 20 check needs key bytes that --verify does not have.
  • D14, minor. Writing rules.

• Design, round 2: DESIGN: ACCEPT

  • D15, major. Rule 8. Operator keys read the key file at each use.
  • D16, minor. Rule 3 and 16. Hard links and bind mounts, and a key that appears during the copy.
  • D17, minor. Rule 7. The list of earlier writes is not complete.
  • D18, minor. Rule 22. Cross references.
  • D19, minor. Rule 21. docker exec sees the secret.

• Implementation, round 1: IMPLEMENTATION: REJECT

  • I1, major, wording-only. The systemd drop-in is for a system unit, and flats install makes a user unit.
  • I2, minor, wording-only. The message of an unopenable external key does not say that a second run does not clear it.
  • I3, minor. A fixed failure and a transient failure together give a RerunError.
  • I4, minor. core.New returns the key refusal with status 1.
  • I5, minor, wording-only. backup-key and reset lack the move-away message for an unusable configured key.
  • I6, minor. The read of the key at step 13 repeats the checks of the preflight.
  • I7, minor, wording-only. The Docker steps leave the old container.
  • I8, minor, wording-only. The Compose steps.
  • I9, minor, wording-only. "Move that file away" when the file is the configured key itself.
  • I10, minor, wording-only. Messages that name secret.key with the setting on.
  • I11, minor. Tests that the criteria ask for are missing.
  • I12, minor, wording-only. Rule 12 of host-backup.md.

• Implementation, round 2: IMPLEMENTATION: ACCEPT
  Round 2 closed I1 to I12 and added:

  • R1, minor, wording-only. chmod after chown fails for a user that is not root.
  • R2, minor, wording-only. The output of reset still gives a shell line on Windows.
  • R3, minor, wording-only. One sentence of rule 21 has two subjects.

• Testing, round 1: TESTING: ACCEPT

  • T1, minor. The second mutation run has no log.
  • T2, minor, wording-only. The -race run and the delta do not match in the report.
  • T3, minor. backup-key has no test for a second key or a resolved link.
  • T4, minor. key ls and key rm are not tested in a rule 3 state.
  • T5, minor. No test calls the console operator-key methods with the setting on.
  • T6, minor, wording-only. One test description in the report overstates.

• Testing, round 2: TESTING: ACCEPT
  Round 2 closed T2 to T6, left T1 open as a report defect, and added:

  • U1, minor. The package lacked the two mutation logs that the report cites (I had not copied them). They are in the evidence now.
  • U2, minor, wording-only. The report said 14 skips; the inventory has 15.

• Impact assessment after the CI code scanning failure: IMPACT: ACCEPT

  • No finding. One optional note: the evidence gave the CI outcome without a run link.

CI

The first CI run failed two checks. CodeQL reported 5 go/path-injection alerts on the file calls of the configured key path. The value had been cleaned
with filepath.Clean(p); host.data_dir is cleaned with filepath.Clean("/" + p) (cleanDataDir), the form that the analysis accepts, so cleanKeyPath now
calls cleanDataDir. For a validated absolute path the result is the same string, and the impact assessment above confirmed it. The second run of CI
on that commit passes all 25 checks and lists no open alert in this change. test (other) also failed once, in TestDocsRealRuntimeEndToEnd (TempDir RemoveAll cleanup: directory not empty), and passed on the next run. The push run of main after #238 failed the same job in another test of the package
(TestProxyEndToEnd/TLS_proxy_with_X-Forwarded-Proto, read: EOF). I did not find the cause of either, and neither touches the key.

Dispositions: every finding was accepted and corrected. Optional suggestions were not all adopted. Not adopted: a read-only row for the setting in
the console's Settings view (out of scope in the design), merging backup.isUnder with config.InsideDir (it changes --out on Windows, so it is
its own change), a sentence in docs/agent/topics/env-secrets.md (true with the default path, pinned by tests), the order of the "nothing to reset"
check of reset, a check per case of the message in one start test, and a host-level reload test. The corrections that came after an accepted
round are covered by the next round, except the wording corrections R1 to R3 of the implementation round 2 and the report corrections of
the testing stage, which the testing round 2 read.

The mutation checks of the oracles found two weak tests, now fixed: reset without its check of a second key, and restore-key --out below the
data directory. They also show that two checks of a key that resolves into the data directory are covered on Linux only, because Windows skips the link tests
without a privilege.