Follow up on macOS Keychain secret-store review findings #440

Source review: #436 (review)

PR: #436

The review approved the macOS Keychain secret-store change, but identified two non-blocking follow-ups that should be tracked separately.

Follow-ups:

1. Add explicit newline rejection for the ACQ_SECRET_TEST_VALUE test escape hatch.

The shipped production paths are structurally safe today because secret values are read with single-line input, and acq_secret_import already rejects newline and TAB values through the storable-value validation path. The review found that the test-only ACQ_SECRET_TEST_VALUE branch has no equivalent explicit guard. If a future change accidentally wires a production secret through that branch, an embedded newline could be silently truncated or become a second command in the security -i command stream.

Acceptance criteria:

• ACQ_SECRET_TEST_VALUE rejects embedded newlines explicitly before any Keychain write path is reached.
• The behavior is covered by a focused offline test.
• The validation pattern stays aligned with the existing storable-value rejection used by acq_secret_import.

2. Document or otherwise address the macOS Keychain read-side caveat for control characters and non-ASCII bytes.

The review found that security find-generic-password -w can hex-mangle read-back output for values containing control characters or non-ASCII bytes. The write itself was verified byte-correct through the raw hex dump path, but the normal -w read formatting can be lossy for those values. This is low probability for acq secrets, which are expected to be API tokens, but the limitation is not currently captured in the bats stub, probe script, or docs.

Acceptance criteria:

• The limitation is documented in the appropriate macOS Keychain secret-store docs, or the implementation/probe gains a deliberate handling strategy.
• If behavior is changed, add a regression test that captures the intended handling.
• Do not expose real secret values in any test, log, or documentation example.

Security impact:

This is follow-up hardening and documentation for edge cases found during review. The source review concluded these findings do not block #436.