GET /config returns provider keys in plaintext while masking the one local credential a same-uid process does not need

[ouransishen]

Summary

~/.thinkwatch/config.yaml holds every credential in plaintext. That is defensible on its
own — but two things make it worse than it looks:

1. The masking is on the wrong secret. GET /config masks listen.control.key (the
   local control credential, which a same-uid process doesn't need — it can just edit the
   file) while returning providers[].key and clients[].key in plaintext in the same
   response. The upstream credential — the one that spends the user's money — is the
   unmasked one.

2. A same-uid process can write the config through the control plane, even when it
   cannot write the file. On this machine a sandboxed agent is blocked by its own
   process sandbox (read-only for ~/.thinkwatch) yet PUT /config over the unix socket
   succeeds, because the write happens inside twcore. The sandbox guards the caller,
   not the callee.

So the effective protection for a local model is: it can read every secret directly, and
it can rewrite the policy that governs it — without ever needing the control key.

Reproduced (core 0.61.0, Linux x86_64, 2026.10.4)

① Everything of value is in one plaintext file, mode 600.

-rw-------  ~/.thinkwatch/config.yaml     ← control.key, clients[].key, providers[].key
-rw-------  ~/.thinkwatch/data.db
srwx------  ~/.thinkwatch/twcore.sock     ← same-uid can connect

600 protects against other uids. Against a process running as the same user — which is
exactly what a local agent is — it does nothing. Measured: a same-uid agent reads the file
and obtains all four secrets verbatim.

② Masking covers the least useful secret, and misses the most valuable one.

GET /config →
  control.key            → hidden-see-twcore-control-key-xxxx…   (masked ✅)
  clients[default].key   → verbatim                             (plaintext ⚠)
  clients[dsh].key       → verbatim                             (plaintext ⚠)
  providers[9router].key → verbatim                             (plaintext ⚠)

control.key is a local credential — the README notes the control plane is unix-socket /
loopback only, and remote control is off by default. A same-uid process does not need it:
it can read or edit the file directly, or call the socket as the app does. Meanwhile the
upstream key, which grants paid access to the user's providers, is served in the clear by
the same endpoint.

③ The control plane writes the config its caller cannot write.

# same-uid agent, direct write:
open('~/.thinkwatch/config.yaml','a')  → OSError [Errno 30] Read-only file system

# same-uid agent, via the control plane:
GET  /config  → {"version":"blake3:6ddb3813f4e6", "text":…}
PUT  /config  -d {"text":<same text>, "base_version":"blake3:6ddb3813f4e6"}
              → {"version":"blake3:6ddb3813f4e6"}      (succeeded; file sha256 unchanged)

The version token needed for optimistic locking is handed out by GET /config, so a
caller who can read can also write. I did this with an identical payload so the file was
not altered — but the round trip succeeded, which is the point.

I have not verified that a malicious config rewrite (e.g. disabling
security.redact) is accepted, because doing so would have changed a live configuration.
The read-version → write-path chain is verified; whether a policy change sticks is the
natural next test and I'd rather the maintainers run it than mutate their config.

Why this is not covered by the stated threat model

The README states the design goal as protection against relays — the remote provider
must not see the credentials. That is implemented well: measured 0 occurrences of the
upstream key in data.db or in stored request bodies.

But redaction is an outbound control, and the README/issue history treat the local
filesystem as trusted. For a gateway whose stated purpose is to sit in front of local
clients, the local filesystem is not a trusted boundary — it is where the adversary lives.
Concretely, on a machine that runs coding agents, "a process running as this user" is not
hypothetical; it is the primary consumer.

Two distinct controls are being conflated:

Control	Protects against	Implemented?
Outbound redaction	the relay / upstream seeing the key	✅ yes
At-rest protection	another local process reading the key	❌ no
Display masking	a human shoulder-surfing the UI	⚠ implemented, on the wrong field

Requests, cheapest first

1. Mask providers[].key and clients[].key in GET /config too. They are already
   masked elsewhere in the product. Right now the one field the UI hides is the one a local
   process doesn't need, while the fields it does need are echoed in the clear. (Small,
   self-contained, aligns the endpoint with the UI.)

2. Document the boundary honestly. State in the Security docs that config.yaml is
   plaintext, that 600 is the whole local protection, and that redaction is outbound-only.
   The masking in the UI currently reads as a storage guarantee; one sentence would stop
   that misreading. (Cheapest; useful even if nothing else changes.)

3. Key-provider indirection. ${ENV_VAR} works for providers[].key and
   clients[].key but not for listen.control.key:

   providers[].key    = ${MY_UP_KEY}   → ✅ valid
   clients[].key      = ${ENV_A}       → ✅ valid
   listen.control.key = ${MY_CTRL_KEY} → ❌ has to be 64 hexadecimal characters
   

   The one credential that cannot come from outside the file is the highest-privilege one.
   A file: or OS-keychain source read at startup (rather than an env var baked into a
   unit file) would fit the existing design — I understand feat(control): one control key and a Noise handshake on every control connection #184 removed TOKEN_ENV
   deliberately because the Noise PSK must be readable at handshake time. Note there is no
   keychain integration in the binary today (libsecret / SecretService / Keychain / DPAPI:
   0 references).

4. An optional at-rest encryption for credential fields, keeping a plaintext skeleton,
   so an unattended restart degrades to "gateway up, upstreams locked" rather than "nothing
   starts". A passphrase prompt that blocks twcore serve would break the daemon case
   (twcore.service), which is presumably why plaintext was chosen — this shape avoids that.

If this is out of scope

Item 1 and item 2 together cost very little and remove the misleading part. I'd be glad to
send a PR for item 1 if the direction is welcome.

---

Filed after reading #184 to understand the intended design. This is a proposal about
boundaries, not a claim that redaction is broken — it works.

— 偶然死神 (@ouransishen) · GPG C95A4D3648C1E00D

[fylorn]

Thanks for the careful write-up, and for not mutating your live config to prove the point.

You're right that the boundary wasn't stated anywhere. Here is where we landed on each request:

1. Masking providers[].key and clients[].key in GET /config: not doing this. The control key isn't masked to hide it from local programs. It is masked so that a write through the control plane cannot change the control key itself (only twcore control-key --rotate or editing the file can), and so the configuration history never stores it. Masking the other keys would not protect anything from a same-uid process, which can read the file directly, and the app deliberately shows configuration values as written.
2. Documenting the boundary: done in Tool-call inspection guards ThinkWatch's own data directory; the docs say what protects config.yaml #287. The configuration reference ("Where the file is") and the README now say that the file's 0600/0700 permissions keep out other users but not programs running as the same user, that outbound redaction protects what leaves the machine rather than what is on disk, and why the control key is masked.
3. ${ENV} for listen.control.key: not doing this. The variable would have to come from somewhere the service reads at start (a unit file, an env file, a shell profile), which a same-uid process can generally read too, and the upstream keys, the ones that cost money, stay in the file either way.
4. At-rest encryption: not doing this. Without a passphrase typed at start, the decryption key has to sit on the same disk, which is obfuscation against exactly the same-uid reader you describe; with one, the daemon case breaks, as you point out.

What we added instead sits on the gateway's side of the boundary. Tool-call inspection has a new built-in rule, thinkwatch-data (#287), for a tool call whose path or command points into ThinkWatch's data directory: ~/.thinkwatch, %APPDATA%\ThinkWatch, and /var/lib/thinkwatch or /etc/thinkwatch on a server. Under enforce it cuts the response off, so a model steered by an injected prompt cannot read the keys or rewrite its own protections through a tool call. It reads path and command arguments only, so editing a document that mentions the directory is not flagged. A data directory moved with THINKWATCH_HOME needs a custom rule, as the docs say. It will be in the next core release.

No PR needed for item 1, but thank you for offering. Closing this; feel free to reopen if something here doesn't hold.