fix(varlock): enum, url, ip, md5, and port data type fixes by theoephraim · Pull Request #1064 · dmno-dev/varlock

theoephraim · 2026-09-03T07:14:47Z

Split 1 of 3 out of #1061 by @WalksWithASwagger, who wrote the original fixes. Commit authorship is preserved; the three slices touch disjoint files so they review and land independently.

No existing test expectation changes here: every assertion on main still holds, and the new cases are additive.

Fixes

@type=enum: numeric and boolean members now match string values from process.env / overrideValues, so a CI override of LEVEL=2 satisfies enum(1, 2, 3)
@type=url: allowedDomains is matched in full against the URL host, closing a substring hole (below)
@type=url: noTrailingSlash now catches a trailing slash followed by a query string or hash (below)
@type=ip: an over-escaped \. in the IPv6 regex meant the IPv4-mapped branches could never match; ::ffff:192.168.1.1 now validates
@type=md5: accepts uppercase hex, normalized to lowercase
@type=port: rejects non-integers. Unquoted 80.5 is already a number after parse auto-coercion and skipped the existing string check

`allowedDomains`

The value was being run through String.prototype.includes, which is a substring match. Probing main:

form on `main`	result
`allowedDomains="example.com"` → host `example.com`	valid
`allowedDomains="a.com,b.com"` → host `a.com`	valid (by substring accident)
`allowedDomains="a.com,b.com"` → host `evil.com`	`Unexpected error during validation` (`.join` throwing on a string)
`allowedDomains="myexample.com"` → host `example.com`	valid ← the hole
`allowedDomains=[a.com, b.com]`	worked correctly, but had no test coverage

So a host passed an allowlist it was merely a substring of, and a legitimate rejection crashed instead of reporting.

An array is the documented form and is now what we lead with and test, validated the way allowedProtocols directly above it does. A bare string is treated as a single host rather than being split on commas: main's vscode snippet inserted allowedDomains=${1:"example.com"}, so the single-string form is plausibly in the wild and keeps working. Splitting on commas instead would leave two options on the same type behaving differently, since allowedProtocols=postgres already errors with "must be an array of strings".

A comma inside a string now errors and names the fix:

allowedDomains must be an array of strings - use allowedDomains=[example.com, api.example.com]

That is the one intentional break. A comma-string appeared to work on main only through the substring bug, and it now fails loudly with a one-line fix rather than silently changing meaning.

Ports are handled per entry. main compared against url.host, which carries the port, so allowedDomains=[localhost] rejected http://localhost:3000 with Domain (localhost:3000) is not in allowed list: localhost - a likely thing to hit in local dev. Now an entry without a port matches the hostname and allows any port, and an entry that names a port pins it:

`allowedDomains`	value	result
`[localhost]`	`http://localhost:3000/`	valid
`[localhost]`	`http://localhost/`	valid
`["localhost:3000"]`	`http://localhost:3000/`	valid
`["localhost:3000"]`	`http://localhost:9999/`	rejected
`["example.com:443"]`	`https://example.com/`	valid
`["example.com:443"]`	`http://example.com/`	rejected
`[example.com]`	`https://evil.com:8443/`	rejected

An entry naming a port is normalized through the same parser as the value, so a protocol default port is dropped from both sides - otherwise ["example.com:443"] would never match https://example.com, whose host is just example.com. A bracketed IPv6 entry like ["[::1]"] takes the hostname branch and still works.

Wildcards are still unsupported and out of scope here: allowedDomains=["*.example.com"] is taken as a literal hostname and matches nothing. Worth adding later, following the allowWildcard semantics the domain type already has, along with an error for a * entry in the meantime.

`noTrailingSlash`

The original PR exempted a root /, inverting an existing test, on the grounds that the reference docs already promised an exemption ("except root /"). Reverted: the docs sentence was the thing that was wrong.

This option exists so a value is safe to concatenate onto, and https://example.com/ breaks ${MY_URL}/path into //path exactly like any other trailing slash. main's behavior stands, its test is untouched, and the docs plus the vscode tooltip are corrected to describe it.

Kept the part of the original change that was a real fix. main tested whether the whole written value ended in /, so a trailing slash followed by a query slipped through:

value	`main`	here
`https://example.com`	valid	valid
`https://example.com/`	invalid	invalid
`https://example.com/path/`	invalid	invalid
`https://example.com/path/?q=1`	valid	invalid ← fixed

The check drops any query or hash and then looks at the written value. It cannot use url.pathname, since WHATWG normalization gives both https://example.com and https://example.com/ a / path and so cannot tell the two spellings apart.

md5 normalization: checked, not a breaking change

Flagged this for review earlier, then verified it against main. Uppercase and mixed-case md5 values fail validation today:

UPPER isValid: false
UPPER errors: [ 'Value must be a valid MD5 hash string' ]
MIXED isValid: false

No schema with an uppercase md5 loads successfully on main, so the only values whose delivered form changes are ones that previously hard-failed. Purely additive, so the coercion stays.

Coverage

Mutation-tested rather than eyeballed: each fix was broken in turn to confirm a test caught it. Every mutant is killed - reverting the IPv6 escaping, the hostname comparison, the port guard, the empty-entry filter, the trim, the comma-string error, the port integer check, the md5 lowercasing, and either half of the enum coercion each fails at least one test.

That found three uncovered branches (an entry naming a port against a value without one, a default-port entry under another protocol, and the catch for an unparseable port such as ["localhost:99999"]), plus the bracketed IPv6 entry the port guard exists to avoid, mixed bare-and-port entries, trimming and empty entries, a non-array non-string setting, and the hash half of the noTrailingSlash split - which is reachable, since a quoted value preserves #.

Two mutants survived because the code was redundant, not because a test was missing, so both are removed:

md5 validated with a case-insensitive regex that coerce's lowercasing made unreachable
port checked Number.isInteger in both coerce and validate. Kept the coerce-side one, which matches the CoercionError the neighbouring string checks throw

Entries are hosts only. One carrying a path is compared against a host, so it could never match and silently rejected every URL; it now errors and names the host to use:

allowedDomains=["example.com/path"]   →  allowedDomains entries must not include a path - use example.com
allowedDomains=["https://example.com"] →  allowedDomains entries must not include a path - use example.com

A scheme brings // and is caught the same way. This is reported even when another entry in the list would have matched, since a bad entry is a schema mistake rather than a value that failed. A bracketed IPv6 entry holds no slash and is unaffected.

Known gap, not fixed here

Wildcards are unsupported and ["*.example.com"] is still accepted silently as a literal host that matches nothing. Left for the wildcard change, which should follow the allowWildcard semantics the domain type already has.

Test plan

bunx vitest run --root packages/varlock (123 files, 1986 passed)
bunx turbo run build --filter=varlock --filter=env-spec-language
bun run --filter @varlock/website build
bun run lint clean
git diff origin/main on the test file shows no deleted assertions

enum: match numeric/boolean members against string values from process.env and overrideValues. url: treat allowedDomains comma-strings as a host list rather than a substring match, and allow a root `/` under noTrailingSlash. ip: fix an over-escaped `\.` in the IPv6 regex so IPv4-mapped addresses validate. md5: accept uppercase hex, normalize to lowercase. port: reject non-integers, including unquoted 80.5.

github-actions · 2026-09-03T07:15:23Z

The changes in this PR will be included in the next version bump.

Patch releases

@env-spec/parser 0.5.1 → 0.5.2
@varlock/native-helper-darwin 1.18.0 → 1.18.1
@varlock/native-helper-linux-arm64 1.18.0 → 1.18.1
@varlock/native-helper-linux-x64 1.18.0 → 1.18.1
@varlock/native-helper-win32-x64 1.18.0 → 1.18.1
env-spec-language 0.4.0 → 0.4.1
varlock 1.18.0 → 1.18.1

Bump files in this PR

data-type-fixes.md (view diff) (edit)

Click here if you want to add another bump file to this PR

This comment is maintained by bumpy.

github-actions · 2026-09-03T07:15:51Z

📦 Bundle size

⚠️ grows the bundle by 10.0 KB (+0.2%)

Metric	main	This PR	Δ
Total dist	4441.9 KB	4452.0 KB	+10.0 KB (+0.2%)
JS	1689.2 KB	1691.6 KB	+2.4 KB (+0.1%)
Sourcemaps	2644.5 KB	2652.2 KB	+7.6 KB (+0.3%)
Type defs	108.2 KB	108.2 KB	—
Other	0.0 KB	0.0 KB	—

dist/ only; native binaries are versioned separately and not counted here.

cloudflare-workers-and-pages · 2026-09-03T07:17:02Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
🔵 In progress View logs	varlock-website	`f573865`	Sep 03 2026, 06:50 PM

pullfrog

✅ No new issues found.

Reviewed changes across the five changed files, including runtime validation, editor diagnostics, documentation, release metadata, and regression coverage.

Enum coercion: String overrides now resolve to matching numeric and boolean members while preserving exact string matches.
URL validation: Comma-separated domain lists use normalized exact host matching, and root paths are exempt from noTrailingSlash.
IP, port, and MD5 validation: IPv4-tailed IPv6 forms validate, fractional ports fail, and uppercase MD5 values normalize to lowercase.
User-facing alignment: Reference docs and VS Code completion text reflect the runtime contracts, with focused tests covering each fix.

^{｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

An array is the documented form and had no test coverage. Validate it the way allowedProtocols right above it does, erroring clearly when the list holds non-strings, and keep accepting a comma-separated string. Widen the settings type to match, and lead with the array form in the reference docs and the vscode snippet.

pullfrog

Important

The array-first allowedDomains change leaves VS Code diagnostics inconsistent with runtime validation.

Reviewed changes since the prior Pullfrog review at 6c77eb03, focusing on the expanded allowedDomains contract and its user-facing integrations.

Promoted array syntax: Made allowedDomains=[...] the primary documented and suggested form while retaining comma-separated string support.
Hardened runtime validation: Added string-member validation, normalized exact host matching, and regression coverage for arrays, case differences, substrings, and invalid entries.
Updated user-facing guidance: Revised the reference example, release entry, and VS Code completion text for the array-first behavior.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

pkg-pr-new · 2026-09-03T16:44:06Z

Open in StackBlitz

@env-spec/parser

npm i https://pkg.pr.new/dmno-dev/varlock/@env-spec/parser@1064

varlock

npm i https://pkg.pr.new/dmno-dev/varlock@1064

@varlock/native-helper-darwin

npm i https://pkg.pr.new/dmno-dev/varlock/@varlock/native-helper-darwin@1064

@varlock/native-helper-linux-arm64

npm i https://pkg.pr.new/dmno-dev/varlock/@varlock/native-helper-linux-arm64@1064

@varlock/native-helper-linux-x64

npm i https://pkg.pr.new/dmno-dev/varlock/@varlock/native-helper-linux-x64@1064

@varlock/native-helper-win32-x64

npm i https://pkg.pr.new/dmno-dev/varlock/@varlock/native-helper-win32-x64@1064

@varlock/aws-sigv4-plugin

npm i https://pkg.pr.new/dmno-dev/varlock/@varlock/aws-sigv4-plugin@1064

commit: b696217

Splitting a string on commas would make allowedDomains behave unlike allowedProtocols right beside it, which requires an array. A string is one host; a comma in it errors and names the array to write instead. The single-host string keeps working, which is what the vscode snippet used to insert.

pullfrog

Important

The new runtime contract is sound, but the existing VS Code diagnostic parity issue remains unresolved and now also lets comma strings pass static validation when runtime rejects them.

Reviewed changes since the prior Pullfrog review at 47f469b1, focusing on the narrowed allowedDomains string contract and its user-facing guidance.

Preserved single-host compatibility: Treated a bare allowedDomains string as one exact, case-insensitive host to support existing VS Code-generated schemas.
Rejected ambiguous comma strings: Added a targeted validation error that directs multi-host configurations to array syntax, with effective regression coverage.
Aligned guidance: Updated the reference docs, completion text, and release entry to describe the single-string and array behavior.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

noTrailingSlash exists so a value is safe to concatenate onto, and `https://example.com/` breaks `${MY_URL}/path` exactly like any other trailing slash, so the root path is not exempt. That restores main's behavior and its test; the reference docs claimed an exemption the code never had, so the docs and the vscode tooltip are corrected instead. Keeps the part of the original change that was a real fix: the check drops any query or hash first, so `https://example.com/path/?q=1` is caught. It reads the written value rather than url.pathname, which normalizes `https://example.com` to a `/` path and cannot tell the two spellings apart.

pullfrog

ℹ️ No new issues in the latest commit. The prior unresolved review thread still requires attention before approval.

Reviewed changes since the prior Pullfrog review at 1158dd28, focusing on the revised noTrailingSlash contract and its coverage.

Restored strict root handling: Rejected an explicitly written root slash again so https://example.com/ remains invalid while https://example.com remains valid.
Extended suffix detection: Detected a trailing path slash even when a query string or hash follows, without confusing a slash inside the query with a path suffix.
Aligned user-facing guidance: Updated the reference docs, VS Code completion text, release entry, and focused tests to describe and verify the revised behavior.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

The check compared against url.host, which carries the port, so allowedDomains=[localhost] rejected http://localhost:3000. Compare against the hostname instead. An entry that carries a port could then never match, so it errors and names the bare domain to use rather than silently rejecting every URL. The `:\d+$` test leaves a bracketed IPv6 entry like [::1] alone.

pullfrog

ℹ️ No new issues in the latest commit. The prior unresolved VS Code diagnostic parity thread still requires attention before approval.

Reviewed changes since the prior Pullfrog review at 22beb300, focusing on URL hostname matching when values or allowlist entries contain ports.

Ignored value ports: Switched exact allowlist matching from URL.host to URL.hostname, allowing entries such as localhost to match URL values such as http://localhost:3000.
Rejected allowlist ports: Added a targeted validation error for allowlist entries containing numeric ports while preserving bracketed IPv6 hostnames.
Aligned guidance and coverage: Updated reference docs, completion text, release metadata, and focused tests for the hostname contract.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

Comparing against url.host meant allowedDomains=[localhost] rejected http://localhost:3000. An entry without a port now matches the hostname and so allows any port, while an entry that names one pins it - which keeps ["localhost:3000"] working as it did before. An entry naming a port is normalized through the same parser as the value, so a protocol default port is dropped from both sides and ["example.com:443"] matches https://example.com.

pullfrog

Important

The port-pinning implementation accepts non-host authority syntax, which can make an allowlist entry authorize a different host than its text suggests.

Reviewed changes since the prior Pullfrog review at 1297859f, focusing on explicit port constraints in URL domain allowlists.

Added port pinning: Allowed an allowedDomains entry with a numeric port to require that effective port while entries without ports continue to accept any value port.
Normalized default ports: Parsed entries with the value URL's protocol so explicit defaults such as HTTPS port 443 compare with WHATWG-normalized URLs.
Updated guidance and coverage: Revised docs, completion text, release metadata, and runtime tests for matching, mismatched, and default ports.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

…hecks Mutation-tested every fix in this PR by breaking it and checking a test caught it. Three branches had no coverage: an entry naming a port against a value without one, a default-port entry under another protocol, and the catch for an unparseable port. Nor did the bracketed IPv6 entry the port guard exists to avoid, mixed bare and port entries, trimming, a non-array non-string setting, or the hash half of the noTrailingSlash split, which a quoted value does reach. Two mutants survived because the code was redundant, not because a test was missing. md5 validated with an /i regex that coerce's lowercasing made unreachable, and port checked Number.isInteger in both coerce and validate. Kept the coerce-side check, which matches the CoercionError the neighbouring string checks throw.

pullfrog

ℹ️ No new issues in the latest commit. The two prior unresolved threads concerning malformed authority syntax and VS Code diagnostic parity still require attention before approval.

Reviewed changes since the prior Pullfrog review at d12454ce, focusing on expanded URL edge coverage and removal of redundant checks.

Expanded URL coverage: Added regression cases for port mismatches, protocol-sensitive defaults, mixed allowlist entries, case normalization, bracketed IPv6, malformed ports, normalized entries, invalid option types, and hash suffixes.
Removed duplicate validation: Kept integer enforcement in port coercion and lowercase normalization in MD5 coercion while removing checks made unreachable by those coercion steps.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

An entry is compared against a host, so one holding a path could never match and silently rejected every URL. It now errors and names the host to use. A scheme brings `//` and is caught the same way, and a bracketed IPv6 entry holds no slash so it is unaffected. Reported even when another entry would have matched, since the entry is a schema mistake rather than a value that failed.

pullfrog

ℹ️ No new issues in the latest commit. The new path and scheme validation is sound, but it only partially addresses the prior malformed-authority finding; that thread and the VS Code diagnostic parity thread remain unresolved.

Reviewed changes since the prior Pullfrog review at b57d9b08, focusing on stricter allowedDomains schema validation.

Rejected path-bearing entries: Added a targeted validation error for allowlist entries containing paths or schemes, including host suggestions where available.
Validated invalid-list handling: Added coverage showing malformed entries fail even alongside a matching host and that entries without a usable host omit the suggestion.
Aligned user guidance: Updated the reference docs, VS Code completion text, and release entry to describe host-only allowlist entries.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

Handing a whole entry to URL let its authority parser read credentials and swap the host: `trusted.example:443@evil.example:8443` parsed with a host of `evil.example:8443`, so the entry authorized that and rejected trusted.example - the opposite of what it reads as. Entries are now checked against a hostname-with-optional-port shape first, which covers the earlier path and scheme cases too. No host is suggested for a credential-bearing entry, since the one it would have authorized is the attacker's. Both branches now compare through the same parser, so an internationalized entry normalizes to punycode and matches. Also brings the vscode diagnostics back in line with the runtime. They lowercased only the value, so [Example.COM] raised a false error; they compared against the port-bearing host; they still exempted a root slash under noTrailingSlash; and they accepted a comma string the runtime now rejects.

pullfrog

Important

The new host-only and diagnostic parity work still accepts a backslash path in an allowlist entry and reports an empty allowlist differently from runtime.

Reviewed changes since the prior Pullfrog review at 345fa5bc, focusing on the stricter allowedDomains shape contract and VS Code parity.

Restricted allowlist entries: Added a shared host-plus-optional-port shape check that rejects credentials and other URL components before WHATWG parsing.
Aligned editor diagnostics: Mirrored runtime hostname and port matching, comma-string rejection, malformed-entry handling, and strict noTrailingSlash behavior in VS Code.
Expanded regression coverage: Added runtime and editor tests for credentials, malformed ports, bracketed IPv6 port pinning, IDN normalization, case normalization, and trailing-slash handling.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

WHATWG treats a backslash as a path separator for special schemes, so `trusted.example\path` passed the hostname-only shape check and then matched on the host alone. Excluded alongside `/`. An empty list, or one whose entries are all empty, could never match and rejected every URL while naming nothing in the message. It now says the list is empty. The vscode diagnostics skipped matching entirely in that case and accepted every URL, so both sides now share the contract, with an empty quoted string still meaning the option is not set.

pullfrog

ℹ️ No new issues in the latest commit. The prior unresolved VS Code non-string array-member parity thread still requires attention before approval.

Reviewed changes since the prior Pullfrog review at ecf418c2, focusing on the final allowedDomains shape and empty-list handling.

Rejected backslash paths: Excluded backslashes from host entries in runtime and VS Code validation, with regression coverage proving trusted.example\\path is diagnosed.
Defined empty-list behavior: Added an explicit error for empty arrays and arrays containing only empty entries while preserving an empty bare string as an unset option.
Kept user-facing behavior aligned: Updated reference documentation, release metadata, and editor diagnostics to describe and enforce the same contract.

^{｜ Fix it ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

The editor accepted `allowedDomains=[example.com, true]` while a load rejects it, which is the worst shape for a diagnostic to have. Quoting is what decides whether a member is a string, and parseListOption unquotes everything, so this reads the raw array literal instead. Applies to allowedProtocols too, which has the same runtime rule and was silent in the same way.

pullfrog

Important

The editor parity fix still disagrees with the parser for several unquoted scalar forms and should be corrected before merge.

Reviewed changes since the prior Pullfrog review at f5738659, focusing on the new raw-array member validation in VS Code diagnostics.

Detected unquoted scalar members: Added raw-literal inspection so boolean and numeric members rejected by runtime are also diagnosed by the editor.
Extended option parity: Applied the check to both allowedDomains and allowedProtocols while preserving quoted values as strings.
Added focused regression coverage: Verified bare true and 42 are rejected while quoted "true" remains valid.

^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

…oerce The regex I wrote to spot non-string array members disagreed with the parser in six ways: it missed `undefined`, and it flagged `TRUE`, `True`, `00`, `1.0` and unsafe integers, all of which autoCoerce deliberately leaves as strings. So it swapped one wrong diagnostic for another. Export autoCoerce from @env-spec/parser and use it, rather than reimplementing the coercion rules a third time. It bundles into the extension, so no runtime dependency is added. The separate quoting check went with it: members reach autoCoerce with their quotes attached, so a quoted "true" is never the bare literal and the check could not change an outcome.

pullfrog

✅ No new issues found.

Reviewed changes since the prior Pullfrog review at 69e607ff, covering the parser export, editor integration, dependency metadata, and focused regression coverage.

Shared scalar classification: Exported autoCoerce from @env-spec/parser and replaced the editor's approximate regular expression with the parser's exact coercion behavior.
Expanded parity coverage: Verified converted booleans, numbers, and undefined are rejected as non-string array members while case variants, formatted numbers, scientific notation, and unsafe integers remain strings.
Updated package metadata: Added the parser workspace dependency to the VS Code extension and included the parser patch in release metadata.

^{｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

…of the repo Importing @env-spec/parser broke `tsc --noEmit` in CI: the plugin's tsconfig stood on its own with moduleResolution node16, so it resolved the dependency through the exports map to `dist/index.d.mts`, which does not exist until that package is built. It passed locally only because a previous build had left the dist behind. Extend the shared base config, which sets customConditions ts-src and moduleResolution bundler - the same way every other package here resolves a workspace dep to its TypeScript source, with no build needed first. Verified by deleting the parser's dist and typechecking.

pullfrog

✅ No new issues found.

Reviewed changes since the prior Pullfrog review at 1eef09b7, covering the shared TypeScript configuration and workspace dependency metadata.

Aligned compiler resolution: Extended the VS Code package from the shared TypeScript base so tsc --noEmit uses bundler resolution and the ts-src condition to resolve workspace source without prebuilt parser output.
Declared configuration dependency: Added @varlock/tsconfig to the package and lockfile so clean workspace installs provide the inherited config.

^{｜ View workflow run ｜ Using azure/gpt-5.6-sol ｜ 𝕏}

github-actions Bot added maintainer core:website core:varlock core:vscode-plugin labels Sep 3, 2026

theoephraim mentioned this pull request Sep 3, 2026

fix(varlock): type coercions, imported @currentEnv, and leak-scan end hang #1061

Closed

1 task