feat(components): protect .env files in the operations API#1527
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces new operations (get_env_keys, set_env_value, and delete_env_value) to securely manage .env files, including value masking for protected environment files and validation schemas. The reviewer identified three critical issues: first, the setEnvValue and deleteEnvValue operations in operation_authorization.ts are missing their API name mapping, which will block them for users with restricted operation allowlists; second, getEnvKeysValidator lacks project name validation, exposing a potential path traversal vulnerability; and third, sensitive secrets from set_env_value requests are at risk of being leaked in plain text within the operation logs because value and values are not excluded from logging.
|
Reviewed; no blockers found. |
f04f5a8 to
b58bbe7
Compare
kriszyp
left a comment
There was a problem hiding this comment.
Are we going to add the ability to manage env values through Fabric (bypassing op API for security) and an op API mechanism?
|
Two questions:
|
|
@kriszyp I'd also say that, since we already support .env files as a part of components, this PR is a good enhancement for the platform. It protects the files a bit, and lets us build out a nice UX that will feel good for people. That same web-side component can drive what you're talking about with secrets set at a higher level that have real protections. So we could abandon #1528 if that's not the direction you want us to go, and proceed with this PR. |
|
@kriszyp I removed the set_component_file protections, I can draft a Studio PR for this too so we can visualize it better if you'd like. |
Add editor-facing protection for component .env files so secrets aren't
accidentally revealed or clobbered through the operations API, while the
runtime (resources/loadEnv.ts, untouched) still loads the real values.
New operations (all super_user):
- get_env_keys: returns key names only, never values
- set_env_value: upsert one (key+value) or many (values{}), merge-preserving
- delete_env_value: remove one (key) or many (keys[]), merge-preserving
Modified operations:
- get_component_file: returns a masked, value-free body + key list for a
protected .env file
- set_component_file: refuses protected .env files (would clobber other
secrets / write back masked placeholders); points at set_env_value
- get_components: flags protected .env entries with protected: true
Template files (.env.example/.sample/.template, case-insensitive) are not
secret and are exempted from all protection. .env matching is
case-insensitive so a stray .ENV can't slip through unprotected.
Core parsing/serialisation lives in a new pure utility/envFile.ts, built
around dotenv 16's exact parse semantics so written values round-trip and
reported keys match what the runtime loads. Env-key charset is locked to
[\w.-] to prevent key injection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- operation_authorization: register set_env_value / delete_env_value with their snake_case api_name so roles with an `operations` allowlist can actually permit them (otherwise opApiName resolved to the camelCase function name and was always denied) - operationsValidation: validate get_env_keys `project` against PROJECT_FILE_NAME_REGEX to prevent `..` path traversal when joined to the components root in resolveEnvFilePath - serverUtilities: exclude `value`/`values` from the operation-log body so secrets passed to set_env_value aren't written to logs in plain text Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dotenv treats single-quoted values literally, so a value ending in `\` (e.g. a Windows path `'C:\Users\name\'`) closes at that quote. closesQuote was skipping the char after a backslash for all quote types, so it missed the closing quote of such a value and mis-read following keys as continuation lines — corrupting them in upsertEnvValues/removeEnvKeys. Restrict backslash-escaping to double/backtick quotes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dotenv v16 does no backslash expansion inside backtick-quoted values (same as single quotes), so a backtick value ending in `\` closes at that quote. Exclude backticks (not just single quotes) from backslash-escaping in closesQuote so following keys aren't mis-read as continuation lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Remove the set_component_file block on .env files. We aren't worried about clobbering: the Studio UI (the one client) will detect the protected-file mechanism (the `protected` flag, plus get_env_keys / masked get_component_file) and handle .env editing, while power users can still write .env files directly via set_component_file if they choose. Read-side protection is unchanged: get_component_file still masks values and get_components still flags `.env` entries `protected: true`, so clients can detect them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ff86a50 to
adf8731
Compare
This looks good.
Yes, I think that makes sense.
Why? I really liked #1528 and am already trying to build other PRs on it (using its security mechanism), why close it? |
kriszyp
left a comment
There was a problem hiding this comment.
This looks good. Note, that I think this should go in 5.2 (and I assigned that milestone).
What & why
Component
.envfiles are exposed verbatim through the operations API:get_component_filereturns the raw bytes (secrets and all) andget_componentslists them, so anyone browsing a component in the editor/Studio can read every secret value.This adds editor-facing, smoke-and-mirrors protection on the read path, plus merge-preserving write operations — enough to prevent accidental disclosure, not a real security boundary (you can still
console.log(process.env)at runtime). The runtime loaderresources/loadEnv.tsis deliberately untouched, soconfig.yaml/components still load the real values.New operations (all
super_user)get_env_keys{ file, keys, size, mtime }), never valuesset_env_valuekey+value) or many (values:{}) — merges in place, preserving other keys, comments and formatting; creates the file if absentdelete_env_valuekey) or many (keys:[]), leaving the rest intactModified operations
get_component_fileon a protected.env→{ protected: true, keys, message }wheremessageis a value-freeKEY=********rendering instead of raw secrets.get_components→ tags protected.enventries withprotected: true, so clients can detect them.set_component_fileis intentionally not restricted: it writes.envfiles verbatim like any other component file. A client (the Studio UI) detects a protected.envvia theprotectedflag /get_env_keysand decides how to edit — the merge-preservingset_env_value, or a fullset_component_filewrite if the user wants it. (An earlier revision hard-blockedset_component_fileon.env; that block was removed — see the "allow set_component_file to write .env files" commit.)Notes / decisions
get_component_file/get_componentsnever reveal values) and on the convenience of safe edits (set_env_value/delete_env_value). Whole-file writes stay open; clients decide..env.example,.env.sample,.env.template, case-insensitive) are not secret and are exempted from masking/flagging — read verbatim..envmatching is case-insensitive so a stray.ENVisn't masked-around (favors over-protection on case-insensitive filesystems).utility/envFile.ts, built around dotenv 16's exact parse semantics so written values round-trip and reported keys match what the runtime loads. Values that need it are quoted (a value containing both'and"is unrepresentable via dotenv round-trip and is rejected rather than silently corrupted).[\w.-]to prevent key injection (e.g. a key with a newline writing extra assignments).get_*/set_*names fall outside the default read-only MCP allow-list, matching the existing posture forget_component_file.Tests
Plain-
assertsuites (no sinon/rewire):unitTests/utility/envFile.test.js— pure parse / mask / upsert / remove logic + round-trips throughdotenv.parse(proves the runtime still reads the real value)unitTests/components/envOperationsValidation.test.js— validator shape, xor logic, key-injection rejectionunitTests/components/envOperations.test.js— handlers end-to-end against a temp components root (masking, merge-preserving set / delete, template pass-through, andset_component_filewriting a.envverbatim)oxlint, prettier, and
tscpass; the env suites (65 tests) are green.🤖 Generated with Claude Code
Related PRs — env-secret encryption
One
enc:v1:client contract across tiers (public key + envelope; values encrypted before they leave the client):.envprotection in the operations API (masking +get_env_keys/set_env_value/delete_env_value)enc:v1:decrypt hook + envelope contract (stacked on feat(components): protect .env files in the operations API #1527)get_secrets_public_key, runtime decryptor)ClusterSecretsops (key/secrets never on the container)enc:v1encryption)