Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
- **Sandbox volume confinement** (`internal/sandbox/sandbox.go`) — extra `--sandbox-volume` host paths must resolve to a location under the working directory, cannot contain `..` or symlink escapes, and cannot match sensitive prefixes such as `/etc`, `/proc`, `/sys`, `/dev`, `/root`, `/home`, `/var`, `/run`, or `/var/run/docker.sock`.
- **Sandbox read-only enforcement** (`cmd/odek/sandbox_file.go` + `cmd/odek/file_tool.go` + `cmd/odek/perf_tools.go`) — when a sandbox container is active, `write_file`, `patch`, and `batch_patch` translate host paths to `/workspace/...` and copy data into the container with `docker cp`, so a read-only workspace mount (`--sandbox-readonly`) is enforced for the agent's own file tools.
- **Project config sensitive-field rejection** (`internal/config/loader.go`) — `./odek.json` is untrusted, so `base_url`, `api_key`, `system`, the `dangerous` section, `embedding`, `memory`, `sessions`, `skills.dirs`/`skills.embedding`, `telegram`, and `web_search` set there are ignored (with stderr warnings). Project-level sandbox knobs (`sandbox_env`, `sandbox_image`, `sandbox_network`, `sandbox_volumes`) are not silently ignored either; they are gated by the project-level sandbox approval flow before application. These can only be configured from operator-controlled sources: `~/.odek/config.json`, `ODEK_*` env vars, or CLI flags.
- **MCP subprocess environment sanitisation** (`internal/mcpclient/client.go`) — MCP server children receive only a minimal allowlist of safe environment variables plus explicit `env` overrides. Keys matching secret patterns (`*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, etc.) are stripped, preventing a compromised or malicious MCP server from reading parent secrets.
- **MCP subprocess environment sanitisation** (`internal/mcpclient/client.go`) — MCP server children receive only a minimal allowlist of safe environment variables plus explicit `env` overrides. Keys matching secret patterns (`*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, etc.) are stripped, preventing a compromised or malicious MCP server from reading parent secrets. Names are normalised (uppercased, `-`/`_` stripped) before matching, so non-underscore spellings such as `API-KEY` or `APIKEY` cannot slip a secret through the override path.
- **MCP `tools/list` metadata hardening** (`internal/mcpclient/client.go`, `cmd/odek/mcp_approval.go`, `cmd/odek/main.go`) — tool names from MCP servers are validated (ASCII letters/digits/`-`/`_`, ≤ 64 chars) and descriptions are scanned for injection patterns. Tools whose raw names collide with odek built-ins are rejected. Each tool from a project-level server requires per-tool approval (interactive, `ODEK_APPROVE_MCP=1`, or persisted in `~/.odek/mcp_tool_approvals.json`); global servers from `~/.odek/config.json` are operator-trusted. Approval keys hash the server's `env` map (sorted key/value pairs), and the interactive prompt prints each env variable with its value, so a later `env` change cannot silently reuse an old approval.
- **Read-only perf-tool file-size cap** (`cmd/odek/perf_tools.go`) — `count_lines`, `checksum`, `head_tail`, and `word_count` reject files larger than 10 MiB before scanning/hashing, consistent with other perf tools.
- **Inline content size cap** (`cmd/odek/perf_tools.go`) — `base64` and `tr` reject inline `string`/`content` arguments larger than 10 MiB, preventing prompt-injected multi-hundred-megabyte payloads from OOMing the process.
Expand Down
2 changes: 1 addition & 1 deletion docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ Go's `net.IP.IsPrivate()` covers RFC1918 and RFC4193 private ranges, but it does

### 19. MCP server environment sanitisation

MCP server subprocesses no longer inherit the full odek process environment. They receive only a minimal allowlist of safe variables (e.g. `PATH`, `HOME`, `LANG`, `TMPDIR`) plus any explicit `env` overrides from the server config. Keys matching secret patterns — `*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_CREDENTIAL`, `*_PRIVATE_KEY`, etc. — are stripped even when listed in `env`. This prevents a compromised or malicious MCP server from reading secrets loaded from `~/.odek/secrets.env` or other provider keys that were present in the parent environment.
MCP server subprocesses no longer inherit the full odek process environment. They receive only a minimal allowlist of safe variables (e.g. `PATH`, `HOME`, `LANG`, `TMPDIR`) plus any explicit `env` overrides from the server config. Keys matching secret patterns — `*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_CREDENTIAL`, `*_PRIVATE_KEY`, etc. — are stripped even when listed in `env`. Matching normalises each name first (uppercased, `-` and `_` removed), so non-underscore spellings like `API-KEY` or `APIKEY` — both legal in environment variable names — cannot bypass the filter through the override path. This prevents a compromised or malicious MCP server from reading secrets loaded from `~/.odek/secrets.env` or other provider keys that were present in the parent environment.

### 20. Schedule file atomic-write hardening

Expand Down
13 changes: 9 additions & 4 deletions internal/mcpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,18 @@ var allowedEnvVars = map[string]bool{
// isSensitiveEnvVar reports whether a key looks like a secret. These patterns
// are blocked from being forwarded to MCP children even if they are present in
// the parent environment or explicitly supplied as overrides.
//
// The name is normalised (uppercased, "-" and "_" stripped) before matching so
// non-underscore spellings — API-KEY, APIKEY, Private-Key — cannot slip a
// secret past the filter; environment variable names legally contain hyphens
// or no separator at all.
func isSensitiveEnvVar(key string) bool {
upper := strings.ToUpper(key)
norm := strings.NewReplacer("-", "", "_", "").Replace(strings.ToUpper(key))
for _, pat := range []string{
"API_KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL", "CREDS",
"PRIVATE_KEY", "ACCESS_KEY",
"APIKEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL", "CREDS",
"PRIVATEKEY", "ACCESSKEY",
} {
if strings.Contains(upper, pat) {
if strings.Contains(norm, pat) {
return true
}
}
Expand Down
32 changes: 30 additions & 2 deletions internal/mcpclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,22 +417,50 @@ func TestBuildEnv_OverridesCannotInjectSecrets(t *testing.T) {
defer func() { osEnviron = orig }()

result := buildEnv(map[string]string{
"LEGIT_VAR": "ok",
"LEGIT_VAR": "ok",
"EVIL_API_KEY": "sk-stolen",
"BOT_TOKEN": "tok-stolen",
// Non-underscore spellings: env var names legally contain hyphens or
// no separator, and must be caught by the normalised matching.
"API-KEY": "sk-stolen",
"APIKEY": "sk-stolen",
"PRIVATE-KEY": "pem-stolen",
"SECRET-KEY": "shh",
})
m := envToMap(result)

if m["LEGIT_VAR"] != "ok" {
t.Errorf("LEGIT_VAR = %q, want ok", m["LEGIT_VAR"])
}
for _, k := range []string{"EVIL_API_KEY", "BOT_TOKEN"} {
for _, k := range []string{"EVIL_API_KEY", "BOT_TOKEN", "API-KEY", "APIKEY", "PRIVATE-KEY", "SECRET-KEY"} {
if _, ok := m[k]; ok {
t.Errorf("sensitive override %q should be dropped", k)
}
}
}

func TestIsSensitiveEnvVar_NormalizesSeparators(t *testing.T) {
blocked := []string{
"MY_API_KEY", "API-KEY", "APIKEY", "Api-Key",
"PRIVATE-KEY", "PRIVATEKEY", "ACCESS-KEY", "ACCESSKEY",
"SECRET-KEY", "SECRETKEY", "AUTH-TOKEN", "AUTHTOKEN",
"DB-PASSWORD", "PASSWORD", "AWS-CREDS", "CREDENTIALS",
}
for _, k := range blocked {
if !isSensitiveEnvVar(k) {
t.Errorf("isSensitiveEnvVar(%q) = false, want true", k)
}
}
allowed := []string{
"PATH", "HOME", "EDITOR", "NODE_ENV", "MCP_SERVER_NAME", "KEYSTONE",
}
for _, k := range allowed {
if isSensitiveEnvVar(k) {
t.Errorf("isSensitiveEnvVar(%q) = true, want false", k)
}
}
}

func envToMap(env []string) map[string]string {
m := make(map[string]string, len(env))
for _, e := range env {
Expand Down
Loading