Skip to content

Commit 831f79f

Browse files
feat(secret): ${file:/absolute/path} references (Spec 111 FR-030) (#1717)
## Description Second of the two PRs for #1706 (Spec 111 FR-030), on top of #1716 (merged). Closes #1706. The path allow-list is the separate follow-up #1721. Adds the `file` provider next to `env` and `keyring`. It is registered in `secret.NewResolver`, so `${file:/absolute/path}` works everywhere the existing references do: an upstream server's `env`, `args` and `headers` (resolved on a copy, so the stored config keeps the reference), `data_dir`, and the proxy's own `api_key`. - **Read rules** are `secret.ReadFile`'s from #1716: absolute path, symlinks followed but the target must be a regular file, 64 KiB cap, one trailing LF or CRLF removed (an LF file and a CRLF file resolve to the identical value), NUL rejected, errors name the reference and reason and never the content. An empty file is an error, as an empty env var is for `${env:}`. - **Read each resolve**, nothing cached, so a server picks up a rotated secret the next time it connects. - **Servers** keep the existing behaviour: a missing file is a missing-secret error naming the reference and the proxy stays up (US7 scenario 2). - **`api_key`**: a reference is resolved at load as a process-only override, so the process runs with the secret while every save, including the one a `PATCH /api/v1/config` triggers, writes the reference back and never the secret. One that does not resolve fails the load and never falls back to a generated key. A key from the environment overrides the file's, and the reference is then not read. Editing `api_key` itself is still persisted as the operator's edit. - Spec 111 FR-030, docs (`keyring-integration.md`, `upstream-servers.md`, `environment-variables.md`) and CHANGELOG updated. @Dumbris, one security question. `${file:}` reads any file the proxy process can read, so a server added through the `upstream_servers` tool with `env: {X: "${file:/data/mcp_config.json}"}` could hand that file to the server's process. `${env:}` already has the same exposure (it can name `MCPPROXY_API_KEY`), servers added that way are quarantined, and `allow_server_add` gates the tool, so I treated it as the same trust boundary and documented it. Would you like a path allow-list (for example a `secret_file_dirs` setting that defaults to `/run/secrets`, `/run/credentials` and the data directory's secrets folder) in this PR, or as a follow-up? ## Testing - [x] I have tested these changes locally - [x] I have added/updated tests that prove my fix is effective or my feature works - [x] All existing tests pass `go test ./internal/config ./internal/secret` pass. New tests cover the provider, LF/CRLF equality, error text without content, `api_key` resolving as an override and surviving a save and a `PersistableConfig` round trip, an edit being persisted, an unresolvable reference failing the load with the file untouched, an env key winning without reading the reference, and server `env`/`headers`/`args` expanding on a copy. By hand with a built binary: a reference in `api_key` authenticates, `PATCH {"tools_limit": 21}` is saved with `api_key` still the reference, a rotated file works after restart, and a missing file refuses to start.
1 parent f13cb22 commit 831f79f

12 files changed

Lines changed: 490 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ Releases follow [Semantic Versioning](https://semver.org/).
101101
document per profile, in mcpproxy's own profile format. Also `GET /api/v1/access/suggest` and the
102102
`suggest` operation of the `profiles` MCP tool (administrators only).
103103
- **`mcp_reject_query_apikey` refuses query-parameter credentials on MCP endpoints.** Set it to `true` and every MCP endpoint (`/mcp`, `/mcp/p/<slug>`, `/mcp/all`, `/mcp/code`, `/mcp/call` and the legacy `/v1/tool_code` aliases) answers a request carrying `?apikey=` with a 401 instead of honouring it, which forces every client onto `Authorization: Bearer` or `X-API-Key`. It cannot stop a URL that was already sent from being logged by whatever sits in front of the proxy; it makes sure such a client stops working until it is migrated, so the token stops appearing in access logs, proxy logs and browser history from then on. The refusal is explicit rather than "no credential" because, with `require_mcp_auth` off, an unrecognised credential would otherwise be served as the anonymous administrator. Default `false`, so nothing changes unless you opt in; the REST API, the Web UI and its `/events` stream are not affected. Live, no restart. **Caveat:** the Connect wizard writes Codex (and the generic fallback) entries with `?apikey=<credential>` in the URL, so a client connected that way gets a 401 once the option is on: remove the query parameter and carry the credential in a header instead (`env_http_headers` or `http_headers`, see the Connect clients guide); Connect itself is not yet aware of the option.
104+
- **secrets: `${file:/absolute/path}` references (#1706, spec 111 FR-030).** A secret can be read from a file, as Docker/Compose secrets, Kubernetes secret volumes and systemd `LoadCredential=` provide it, wherever `${env:}` and `${keyring:}` are accepted: an upstream server's `env`, `args` and `headers`, `data_dir`, and the proxy's own `api_key`. The path must be absolute, a symlink must lead to a regular file, at most 64 KiB is read, one trailing LF or CRLF is removed, and a missing, unreadable or empty file is an error that names the reference, never the content. A server shows a missing-secret error and the proxy stays up; an `api_key` reference that does not resolve stops startup (no key is generated over it). The stored config keeps the reference: the value is resolved on a copy, and the `api_key` secret is a process-only override that no save, including `PATCH /api/v1/config`, writes back. (refs #1706)
105+
104106
- **config: `MCPPROXY_API_KEY_FILE` reads the API key from a file (#1706).** The `*_FILE` form of Docker Compose/Swarm secrets, Kubernetes secret volumes and systemd `LoadCredential=`: the key is not in the process environment (`docker inspect`, `/proc/<pid>/environ`) and is never written to `mcp_config.json`. The path must be absolute; one trailing LF or CRLF is removed; at most 64 KiB is read. Setting both `MCPPROXY_API_KEY` and `MCPPROXY_API_KEY_FILE` (both non-empty) is an error: mcpproxy refuses to start with a message naming both variables (never a key or file contents), and the CLI commands and the tray stop the same way. An empty value is treated as unset. A file that is named but cannot be used stops startup with an error naming the variable and path: mcpproxy does not fall back to the config file or a generated key. `mcpproxy status`, the other CLI commands and the tray read it too, and a tray-launched core reads the file itself instead of receiving the key in its environment. `Config.EnsureAPIKey` now also returns an error. (refs #1706, spec 111 FR-014/FR-030/US7)
105107

106108
- **catalog:** a catalog source whose live search times out or fails is now answered from the listing

‎docs/configuration/environment-variables.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ from the file at start, so it is not in the process environment (`docker inspect
5050
variables: `MCPPROXY_API_KEY and MCPPROXY_API_KEY_FILE are both set; set only one of them`. The
5151
check does not open the file, and the error never contains a key or file contents. An empty
5252
variable counts as unset.
53+
- The same file can instead be referenced from the config as `"api_key": "${file:/run/secrets/mcpproxy_api_key}"`
54+
(see [File references](../features/keyring-integration.md#file-references)).
5355
- A file that is named but cannot be used (missing, unreadable, not absolute, too large, empty) stops
5456
startup with an error that names the variable and path. mcpproxy does **not** fall back to
5557
the config file or a generated key, so a typo in a secret path cannot start the

‎docs/configuration/upstream-servers.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,14 @@ endpoint that backs the "Convert to secret" affordance.
304304

305305
### Secret references
306306

307-
`headers` and `env` values support two reference shapes that get resolved
307+
`headers` and `env` values support three reference shapes that get resolved
308308
at request time rather than stored in plaintext:
309309

310310
| Reference | Source | Lifetime |
311311
|---|---|---|
312312
| `${keyring:NAME}` | OS keyring entry called `NAME` | Persists across restarts; managed via the Secrets page or `mcpproxy secrets` CLI |
313313
| `${env:VAR}` | The `VAR` environment variable on the mcpproxy process | Tied to the shell/launcher that started mcpproxy |
314+
| `${file:/absolute/path}` | The contents of a file (Docker/Kubernetes secret, systemd credential), one trailing newline removed | Read each time the server's connection is set up, so a rotated secret is picked up when the server is next restarted |
314315

315316
When mcpproxy connects to an upstream it substitutes the reference with
316317
the actual secret. The reference itself never reaches the upstream MCP

‎docs/features/keyring-integration.md‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,43 @@ All secrets are stored under the service name **`mcpproxy`**.
4040

4141
## Secret Reference Syntax
4242

43-
MCPProxy supports two types of secret references that can be used anywhere in your configuration where a string value is expected:
43+
MCPProxy supports three types of secret references that can be used anywhere in your configuration where a string value is expected:
4444

4545
| Syntax | Provider | Description |
4646
|--------|----------|-------------|
4747
| `${keyring:name}` | OS Keyring | Resolves from macOS Keychain, Linux Secret Service, or Windows Credential Manager |
4848
| `${env:VAR_NAME}` | Environment | Resolves from environment variables |
49+
| `${file:/absolute/path}` | File | Reads the secret from a file, as Docker/Compose secrets (`/run/secrets/<name>`), Kubernetes secret volumes and systemd `LoadCredential=` provide it |
50+
51+
### File references
52+
53+
`${file:/absolute/path}` keeps a secret out of the config file and out of the process environment
54+
(`docker inspect`, `/proc/<pid>/environ`). The same rules apply wherever it is used:
55+
56+
- The path must be absolute. A symlink is followed (Kubernetes secret volumes are symlinks) but must
57+
point at a regular file. On Windows an absolute path has a drive letter (`C:\ProgramData\mcpproxy\key`
58+
or `C:/ProgramData/mcpproxy/key`); a path that starts with `/` and has no drive is not absolute. In
59+
`mcp_config.json` a backslash must be escaped (`"${file:C:\\ProgramData\\mcpproxy\\key}"`), so forward
60+
slashes are easier.
61+
- At most 64 KiB is read. A larger file is an error, never a truncated secret.
62+
- One trailing newline is removed, LF or CRLF, so a file written on Windows resolves to the same value
63+
as one written on Linux. Leading and inner whitespace are kept.
64+
- A NUL byte, an empty file, a missing file and an unreadable file are errors. The error names the
65+
reference and the reason, never the file's content.
66+
- The file is read each time the reference is resolved, which for an upstream server is when its
67+
connection is set up. A rotated secret is picked up when the server is next restarted.
68+
- The stored config keeps the reference; the value exists only in memory, on a copy.
69+
70+
The proxy's own `api_key` accepts the same reference (`"api_key": "${file:/run/secrets/mcpproxy_api_key}"`),
71+
and so does the `MCPPROXY_API_KEY_FILE` environment variable (see
72+
[API key from a file](../configuration/environment-variables.md#api-key-from-a-file)). An `api_key`
73+
reference that cannot be resolved stops startup: mcpproxy does not generate a key over it. The
74+
reference is resolved when the config is loaded and the key then stays fixed while the process runs (as for
75+
every `api_key` change), so a rotated key file takes effect on the next restart. An API key from the environment overrides the config file's, and then the reference
76+
is not read.
77+
78+
A `${file:}` reference reads any file the mcpproxy process can read, so treat who can edit server
79+
configuration as you would for `${env:}`.
4980

5081
References can be used in:
5182

‎internal/config/api_key_env.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config
22

33
import (
4+
"context"
45
"errors"
56
"fmt"
67
"os"
@@ -93,3 +94,64 @@ func ResolveEnvAPIKey() (EnvAPIKeyResult, error) {
9394

9495
return EnvAPIKeyResult{Set: envSet, Key: envKey}, nil
9596
}
97+
98+
// resolveAPIKeyRef replaces an api_key that is a secret reference with the
99+
// value it points at, as a process-only override: the effective config runs
100+
// with the secret, while every save writes the reference (never the secret)
101+
// back, whatever else the save is about (see process_overrides.go).
102+
//
103+
// The reference is resolved here, once per load, and a reference that cannot be
104+
// resolved fails the load. Generating a key instead would start the proxy
105+
// under a credential the operator did not choose and save it over the
106+
// reference.
107+
//
108+
// A key from the environment (MCPPROXY_API_KEY_FILE or MCPPROXY_API_KEY)
109+
// overrides the config file's key, so with one present the reference is left
110+
// as written, unread, for EnsureAPIKey's override to replace.
111+
func resolveAPIKeyRef(c *Config) error {
112+
if !secret.IsSecretRef(c.APIKey) {
113+
return nil
114+
}
115+
envKey, err := ResolveEnvAPIKey()
116+
if err != nil {
117+
return err
118+
}
119+
if envKey.Set && envKey.Key != "" {
120+
return nil
121+
}
122+
resolved, err := secret.NewResolver().ExpandSecretRefs(context.Background(), c.APIKey)
123+
if err != nil {
124+
// The error is deliberately a fixed sentence: ExpandSecretRefs' own
125+
// text quotes the reference, and the api_key field is the one value
126+
// no error or log line may carry. The operator knows which field it is.
127+
return errors.New("api_key: could not resolve the secret reference; check that the referenced file, environment variable or keyring entry exists and is readable")
128+
}
129+
if resolved == "" {
130+
return fmt.Errorf("api_key: the reference resolved to an empty value")
131+
}
132+
// api_key is fixed for the life of the process (a reload pins the running
133+
// value), and this load may be one that never becomes the live config, such
134+
// as the disk re-read of a server restart. Once a reference has been
135+
// resolved, later loads carry the key the process runs with, not what the
136+
// file holds now: recording a rotated value as the process value would make
137+
// the running key look like an edit, and the next save would write it over
138+
// the reference. The file is still read above, so an unusable reference
139+
// fails the load; a rotated key takes effect on the next start.
140+
if running, ok := runningAPIKeyFromReference(); ok {
141+
resolved = running
142+
}
143+
OverrideForProcess(c, FieldAPIKey, OverrideSourceEnv, resolved)
144+
return nil
145+
}
146+
147+
// runningAPIKeyFromReference returns the api_key this process runs with when it
148+
// was resolved from a secret reference.
149+
func runningAPIKeyFromReference() (string, bool) {
150+
processOverridesMu.RLock()
151+
defer processOverridesMu.RUnlock()
152+
o, ok := processOverrides[overrideKey{FieldAPIKey.Name, OverrideSourceEnv}].(typedOverride[string])
153+
if !ok || !secret.IsSecretRef(o.loaded) {
154+
return "", false
155+
}
156+
return o.process, true
157+
}

0 commit comments

Comments
 (0)