Reject sensitive values redaction cannot protect - #1057
Draft
theoephraim wants to merge 6 commits into
Draft
Conversation
theoephraim
marked this pull request as draft
September 1, 2026 21:31
Contributor
|
The changes in this PR will be included in the next version bump.
|
Contributor
📦 Bundle size✅ shrinks the bundle by 10.6 KB (−0.2%)
dist/ only; native binaries are versioned separately and not counted here. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
The replacement value-time validation drops the documented builtin exemption, causing user-declared VARLOCK_IS_CI values to fail validation.
Reviewed changes in 79984534, including the stricter sensitive-value validation, containment checks, test updates, migration guidance, and release note.
- Redactability validation: Consolidates scalar, composite, and short-value checks after coercion and promotes inherited violations to errors.
- Sensitive containment: Promotes sensitive strings embedded in public values from warnings to errors and shares string-form collection across both sides.
- Current environment: Rejects sensitive
@currentEnvitems regardless of how sensitivity was assigned. - Documentation and tests: Documents the breaking migration and updates sensitivity coverage for explicit and inherited values.
azure/gpt-5.6-sol ｜ 𝕏
| }, | ||
| )); | ||
| } | ||
| if (this.isSensitive) this.checkValueIsRedactable(); |
Contributor
There was a problem hiding this comment.
This now validates builtins even though the previous checkSensitiveIsPlausible() guard and the updated docs exempt them. A schema declaration such as VARLOCK_IS_CI=false inherits sensitivity, reaches this call, and now ends in error with a boolean cannot be sensitive; please preserve the builtin exemption here and add a validation-state regression assertion.
varlock
@varlock/astro-integration
@varlock/cloudflare-integration
@varlock/expo-integration
@varlock/nextjs-integration
@varlock/nuxt-integration
@varlock/vite-integration
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
@varlock/1password-plugin
@varlock/akeyless-plugin
@varlock/aws-secrets-plugin
@varlock/aws-sigv4-plugin
@varlock/azure-key-vault-plugin
@varlock/bitwarden-plugin
@varlock/dashlane-plugin
@varlock/doppler-plugin
@varlock/google-secret-manager-plugin
@varlock/hashicorp-vault-plugin
@varlock/infisical-plugin
@varlock/keepass-plugin
@varlock/keeper-plugin
@varlock/kubernetes-plugin
@varlock/pass-plugin
@varlock/passbolt-plugin
@varlock/proton-pass-plugin
commit: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | bbfdf3c | Commit Preview URL Branch Preview URL |
Sep 02 2026, 06:11 AM |
…stead of failing" This reverts commit ca6b84f.
…cord why errors beat demotion
…igItem own validation-error dedupe
theoephraim
added
the
BREAKING
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE YET - this adds breaking changes that converst some protections we implemented as warnings into errors. We will wait for the next major version since this could be breaking for some folks.
Follow-up to #1054. That PR introduced the rules but kept every one that
@defaultSensitivecould trip as a warning, so nothing inherited from the default could fail a load in a minor release. This is the breaking release those escalations were deferred to.What changes
@currentEnvitemWhy error rather than demote
#1054 said the boolean demotion "belongs in a breaking release," and this branch tried it (
ca6b84f8, reverted ina20280c4). It was the wrong call, for two reasons.The schema stops saying what it means.
@defaultSensitive=trueclaims everything is sensitive; with demotion,PORT=3000quietly isn't. You could no longer read.env.schemaand know what is public without knowing varlock's rules. Erroring forces every non-secret number and boolean to carry an explicit@sensitive=false, which is the state the file should be in. This matters more as agents make most edits: an error with a one-line fix is the best interface an agent can be given, and a silent demotion is the worst. This is now written down as a design principle inAGENTS.md.Demotion needed a lot of subtle machinery to be safe. The effective
dataTypeinfers from whichever source wins,.env.localincluded, so demoting on it would have flipped an untypedAPI_KEY=public (and@static, and inlinable) in exactly one environment when a local placeholder was12345. Making it safe meant a schema-level type getter, a typegen mirror, a new sensitivity source, and a test for the trap - and the first version of the getter was wrong anyway (a default parameter kicked in on an explicitundefined). Erroring handles that trap correctly for free: an inherited-sensitive number is rejected wherever it shows up.The inherited-case error tip points at
@defaultSensitive=falseat the top of the file, since that is the real fix when a hand-written schema has many of these at once.Arrays and objects
A sensitive composite must be made of strings. That's the only rule consistent with how runtime redaction works: it registers each element individually, and only string ones, so a
@type=array(number)has nothing in the map and its elements print in cleartext. The joined form is registered, but that only protects the value printed whole, not any element on its own. So a composite of numbers or booleans is treated exactly like a bare number: demoted when inherited, an error when explicit. A free-formobjector untyped record can't be judged at the type level and falls through to the value-time check.Cleanup
With the explicit/inherited split gone, the schema-time type checks and the value-time leaf check were two mechanisms answering one question. They're now one method,
checkValueIsRedactable, run after coercion on the value that would actually be registered:checkSensitiveIsPlausible, the_sensitiveIsImplausibleflag, and theflag(explicit, implicit)helper are gonestringForms()helper that mirrorsresetRedactionMapexactlySHORT_SENSITIVE_VALUE_LENGTHis no longer imported byenv-graph.tsConfigItem.addValidationError()owns the "safe to re-run" dedupe that the containment check used to do by hand, and both call sites use itNot done here, worth its own PR:
getTypeGenInfo()carries a ~45-line reimplementation ofresolveSensitiveSource(with three deliberate differences: schema-only defs, skipping dynamic decorators, swallowing errors). Extracting a shared purecomputeSensitivity(defs, opts)would remove the duplication, but it is pre-existing behavior with its own subtleties and does not belong in a rules-change PR.One consequence of moving the type checks to value time: an explicitly sensitive
@type=numberwith no value yet doesn't error until a value exists. That's correct (an empty item isn't leaking anything) and the message is the same once one does.Migration
@defaultSensitive=trueis the default, so a hand-written schema with no@defaultSensitiveline now fails on its ports and feature flags. The fix is@defaultSensitive=falseat the top of the file with an explicit@sensitiveon each real secret, which is whatvarlock initgenerates. Schemas frominitare unaffected.Zero fixture changes were needed outside the sensitivity test file itself: #1054 already gave incidental fixtures
@defaultSensitive=false.