feat(varlock): project-configurable audit scan patterns via @auditExtraPatterns - #1075
Conversation
…raPatterns Escape hatch for env access idioms the built-in scanner patterns do not cover: a root decorator taking regex literals whose first capture group is the env key. Patterns run on every scanned file over the same masked content as the built-ins, multiple calls merge additively, and non-regex entries fail loud with a schema error instead of silently scanning nothing. Follows the @auditIgnorePaths plumbing exactly (getRootDecFns reader in audit.command, ScanCodeEnvVarsOptions passthrough, additive-only default call shape), plus docs and vscode intellisense entries.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The documented decorator syntax cannot produce the RegExp values required by the command, so the feature is unusable through an actual .env.schema file.
Reviewed changes across the env-spec decorator registration, audit command integration, scanner behavior, tests, documentation, and VS Code completion metadata.
- Decorator configuration: Adds repeatable
@auditExtraPatternsregistration and collects configured patterns duringvarlock audit. - Scanner extension: Applies custom capture-group patterns to masked content for every supported source language and reports matches as
customreferences. - User surfaces: Documents the new root decorator and exposes it through VS Code completion.
- Coverage: Adds scanner and command tests for forwarding, cross-language matching, nested arrays, and invalid values.
azure/gpt-5.6-sol ｜ 𝕏
…traPatterns env-spec parses decorator function args as strings (never RegExp), and bare literals cannot contain parens/commas — so the documented form could never arrive as a RegExp instance. Accept regex() calls (already RegExp) and quoted '/.../flags' strings via the existing parseRegexLikeString rule; anything else fails loud. Docs now show only surviving forms.
|
One follow-up idea for the backlog (not this PR): letting patterns come from JS imports, so libraries can export them. E.g. our |
There was a problem hiding this comment.
ℹ️ No new issues found in the incremental implementation. The runtime blocker is fixed, but the existing thread remains open because its requested parser-to-command regression coverage is still absent.
Reviewed changes since the prior Pullfrog review, focusing on the corrected env-spec transport and user-facing syntax.
- Accepted parsed transports: Added
regex(...)support through resolvedRegExpvalues and converted quoted slash-delimited strings withparseRegexLikeString. - Updated public syntax: Replaced invalid bare regex examples in the docs and VS Code completion with forms that survive decorator parsing.
- Expanded command coverage: Added a mocked string-transport test and updated validation expectations for unsupported values.
- Verified behavior: Confirmed both documented forms parse and resolve through a real
.env.schema, then produce the expected keys through the builtauditCLI.
azure/gpt-5.6-sol ｜ 𝕏
Starts from real .env.schema text (documented regex() form) through the real graph loader, asserting the scanner receives a usable RegExp and the audit verdict goes green. Mocks stop at the scanner boundary and the process exit — the seam mocked dec.resolve() tests cannot cover.
There was a problem hiding this comment.
Important
The new integration test fails before exercising the parser-to-command behavior it is intended to cover.
Reviewed changes since the prior Pullfrog review, focusing on the new parser-to-command regression coverage.
- Added real-loader coverage: Added a temporary project fixture that loads the documented
regex(...)decorator through the parser and graph before asserting scanner options. - Kept runtime boundaries mocked: Mocked only scanner execution and process exit so the configuration transport remains under test.
azure/gpt-5.6-sol ｜ 𝕏
… fixture The schema text used bare quotes inside the regex() arg, which the grammar rejects outright (SyntaxError before graph loading) — the test failed before exercising anything. Verified against a local peggy build of the real grammar: escaped form parses to a single regex() arg with backslashes intact.
|
Fixed the integration fixture — you were right, it failed before exercising anything: the schema text used bare inner quotes, which the grammar rejects outright ( |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the corrected parser-to-command regression fixture.
- Preserved DSL quote escapes: Updated the generated
.env.schemafixture so its inner single quotes remain escaped and the documentedregex(...)form reaches scanner option assertions through the real parser and graph loader.
azure/gpt-5.6-sol ｜ 𝕏
|
I can certainly imagine varlock plugins contributing new audit patterns. Although those plugins are not usually necessary when using various integrations. Could look for a field in package.json or something but also not quite as foolproof that it actually should be activated. Probably manually setting it is enough, as long as we include them on the various framework/integration docs pages and examples in the varlock-examples repo. |
|
I agree, we can start with just a docs page for every framework on how they can integrate. I can totally create one for Adonis! |
|
@theoephraim should we close #1074 in favor of this one then? |
|
Yeah probably. I’m travelling today but will be able to dig in tonight! |
@auditExtraPatterns could only ever match files whose extension the
built-in scanner already recognizes, since discovery hard-filters on the
language map. A fileTypes=[tf, yaml] option on each call restricts that
call's patterns to those file types, and naming one the scanner doesn't
know also brings those files into the walk - the only way a custom
pattern could reach .tf/.yaml/.sh at all.
Scope is per call: an unscoped pattern still covers the recognized
source extensions and is not extended to file types another call pulled
in, so adding a Terraform rule can't quietly widen existing rules.
Also:
- custom patterns now see string bodies as written (comments are still
masked), so a key that isn't a bare identifier - the NestJS
config.get('app.db.url') case - matches instead of silently missing
- invalid pattern args throw CliExitError rather than a raw SchemaError,
which the CLI's top-level handler re-threw as an unhandled stack trace
There was a problem hiding this comment.
Important
Custom scan patterns can report commented-out references inside template interpolations and should be corrected before merge.
Reviewed changes since the prior Pullfrog review, covering the new file-type scoping and masking behavior.
- Added scoped patterns: Added per-call
fileTypes=[...]configuration and preserved each call's scope through decorator resolution. - Widened source discovery: Allowed scoped patterns to include extensions outside the scanner's built-in language set without extending unrelated patterns or built-ins.
- Changed custom masking: Preserved string bodies for custom patterns while continuing to mask comments for known languages.
- Expanded user surfaces: Documented scoped discovery, updated VS Code metadata, and added command and scanner coverage.
azure/gpt-5.6-sol ｜ 𝕏
The interpolation walker recognized // and /* */ but only stepped past
them without blanking, so a commented-out reference inside ${...} was
reported as a live key. Pre-existing for the built-in patterns; the new
custom-pattern path inherited it, and the docs promise comments are
skipped.
Both walkers now blank interpolation comments through shared helpers,
preserving layout so byte offsets stay valid.
There was a problem hiding this comment.
Important
The interpolation fix introduces a false-negative path for live env references when quoted text contains comment delimiters.
Reviewed changes since the prior Pullfrog review, focusing on the template interpolation masking fix.
- Masked interpolation comments: Added shared line and block comment maskers, applied them inside template interpolations for built-in and custom scans, and added regression coverage for genuine comments.
- Updated release note: Recorded the corrected handling of commented-out references inside template interpolations.
azure/gpt-5.6-sol ｜ 𝕏
The interpolation comment masking added in the previous commit tested for // and /* before stepping over quoted strings, so a URL inside an interpolation blanked the live code after it - and an unterminated "/*" blanked to end of file. Quotes and nested templates are now handled first, matching the string-masking walker's order.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the corrected interpolation masking order.
- Preserved quoted interpolation text: Skipped quoted strings and nested templates before recognizing comment delimiters, preventing URL-like text from masking later live references.
- Added focused regressions: Covered custom and built-in matches following quoted comment delimiters, including nested templates.
azure/gpt-5.6-sol ｜ 𝕏
The audit reference had a tip for false positives but nothing for the opposite case, which is where someone lands when their keys are not being detected.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the new audit CLI documentation pointer.
- Connected audit guidance: Added a troubleshooting tip that directs wrapper-based env access users to
@auditExtraPatterns()and explains whenfileTypes=[...]is needed. - Verified documentation output: Confirmed the generated link targets the decorator reference and the website builds through Turbo with workspace dependencies.
azure/gpt-5.6-sol ｜ 𝕏
@env-spec/parser
varlock
@varlock/astro-integration
@varlock/cloudflare-integration
@varlock/nuxt-integration
@varlock/vite-integration
@varlock/aws-sigv4-plugin
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
commit: |
@auditIgnorePaths matches directory names at any depth, not paths relative to the schema file, so the documented generated/config example silently excluded nothing. Also documents what the scan actually reads (extensions, size cap, always-skipped dirs) and drops an --ignore vendor example that was already covered by the defaults.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the corrected audit scanner documentation.
- Corrected ignore semantics: Documented that
@auditIgnorePaths()and--ignorematch directory names at any depth, including normalization and the unsupported multi-segment case. - Documented scan boundaries: Added the recognized extensions, 1 MB file limit, default ignored directories, nested-project behavior, and built-in access forms.
- Verified documentation output: Confirmed the claims against the command and scanner implementation and built the website through Turbo with workspace dependencies.
azure/gpt-5.6-sol ｜ 𝕏
@auditIgnorePaths and --ignore followed neither of the two things their docs implied: a bare name matched at any depth, and a multi-segment entry matched nothing at all, silently. Both now follow gitignore rules. A bare name still matches at any depth, an entry containing a separator is anchored at the scan root, and a leading / or ./ anchors a single name. Normalization moved into the scanner so the decorator and the CLI flag share one implementation rather than the flag skipping it entirely.
There was a problem hiding this comment.
Important
Anchored exclusions can silently stop working when varlock audit is given positional scan targets.
Reviewed changes since the prior Pullfrog review, focusing on the new directory exclusion semantics.
- Preserved raw configuration: Moved ignore-path normalization into the scanner so decorator and CLI entries share one implementation.
- Added anchored exclusions: Added root-relative multi-segment paths, explicitly rooted single names, trailing-separator handling, and Windows separator support.
- Updated user surfaces: Corrected the CLI and decorator references, VS Code metadata, and release note to describe the expanded behavior.
- Expanded coverage: Added focused traversal tests for bare names, anchored paths, rooted names, subtrees, and separator variants.
azure/gpt-5.6-sol ｜ 𝕏
Inferring the match mode from whether an entry happens to contain a separator meant `docs` and `apps/docs` behaved completely differently with nothing signalling the switch. An entry now says which it wants: bare name matches anywhere, ./path matches that one directory, and a path missing its ./ is an error naming the fix rather than a guess. The normalizer is exported so the CLI validates with the same rule the scanner matches by, and the command tests now partially mock the scanner so they exercise the real normalization.
There was a problem hiding this comment.
Important
The new commit does not resolve the existing positional-target exclusion blocker. An explicit ./apps/docs entry is still forwarded unchanged when apps becomes the scanner cwd, so it is compared with docs and silently fails to exclude that directory.
Reviewed changes since the prior Pullfrog review, focusing on the revised exclusion contract and its effect on the open positional-target concern.
- Required explicit paths: Changed multi-segment exclusions to require
./or/, while preserving bare-name matching at every depth. - Added command validation: Rejected ambiguous path-shaped entries from decorators and
--ignorewith a correction message before scanning. - Updated user surfaces: Revised the CLI docs, root-decorator reference, VS Code metadata, and release note to use the explicit path syntax.
- Expanded regression coverage: Added normalization and command rejection tests; the focused audit suites pass with 51 tests.
azure/gpt-5.6-sol ｜ 𝕏
Adopts @import's prefix convention: ./ and ../ relative to the scanned directory, ~/ for home, or an absolute path. This replaces the gitignore-style reading of a leading / as scan-root-relative, which conflicted with / meaning absolute everywhere else in varlock. Resolved paths are converted to scan-root-relative for matching, and an entry that lands outside the scanned tree is reported rather than silently matching nothing. The docs note that absolute and ~ paths are machine-specific and ./ is the right choice in a committed schema.
There was a problem hiding this comment.
Important
The new containment check rejects some valid in-root absolute and home-relative exclusions.
Reviewed changes since the prior Pullfrog review, focusing on the expanded path forms and containment validation.
- Accepted explicit path forms: Added absolute and
~/exclusion resolution alongside./and../paths. - Rejected ineffective exclusions: Added command validation for paths that resolve outside the project scan root.
- Updated user guidance: Revised the CLI docs, decorator reference, VS Code metadata, release note, and focused exclusion tests for the expanded contract.
azure/gpt-5.6-sol ｜ 𝕏
Two bugs in the path exclusions added earlier on this branch. A relative entry was forwarded verbatim, so with positional scan targets the scanner re-resolved it against each target's cwd: `./apps/docs` with `varlock audit ./apps` looked for `apps/apps/docs` and excluded nothing. Path entries are now canonicalized to absolute before being forwarded. Containment was also a textual compare, so a project reached through a symlink (/tmp on macOS, a linked workspace) rejected an absolute entry that was genuinely inside the tree. Both sides are now compared through realpath as well.
There was a problem hiding this comment.
Important
The latest commit fixes exclusion rebasing for directories below positional targets, but it does not complete the existing positional-target thread: a target that is itself excluded is still scanned. The separate containment issue for valid names such as ..cache is also unchanged.
Reviewed changes since the prior Pullfrog review, focusing on project-root-stable audit exclusions.
- Rebased exclusions: Resolved path exclusions once against the project root and forwarded absolute paths so narrower positional scan roots retain the same meaning.
- Handled symlink spellings: Added realpath-aware containment checks for equivalent raw and canonical project paths.
- Updated documentation and coverage: Clarified project-root-relative semantics and added focused scanner coverage for narrower roots and symlinked paths.
azure/gpt-5.6-sol ｜ 𝕏
…argets
Containment tested relative.startsWith('..'), which also matched valid
directory names like ..cache and hard-errored on them as outside the
tree. Now only ".." itself or a "../" segment counts as escaping.
Naming an excluded directory as a positional scan target also scanned it
anyway, reintroducing the noise the exclusion exists to prevent. The two
instructions contradict each other, so it now errors rather than silently
picking one; a target that merely contains an excluded directory is
unaffected.
65a52bd to
de5bb0b
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the final directory-exclusion corrections.
- Corrected containment: Distinguished the literal
..parent segment from valid in-tree names such as..cache, with focused scanner coverage. - Rejected contradictory targets: Added a project-root exclusion check for positional scan targets while preserving scans of ancestors that merely contain excluded descendants.
- Updated user guidance: Documented that explicitly targeting an excluded directory is an error.
azure/gpt-5.6-sol ｜ 𝕏
Two more silent no-ops in the directory exclusions, found by sweeping the path handling rather than waiting for them in the wild. A path entry pointing at a file was accepted and then excluded nothing, since only directories are pruned from the walk. It now errors and points at @auditIgnore for the "one file's references" case. A path to a directory that does not exist is still fine, because a schema is shared across branches and checkouts. The scan-target check also only consulted user exclusions, so `varlock audit ./node_modules` quietly scanned a directory that is always skipped, and would have blamed @auditIgnorePaths if it had fired. The built-in skip list is now checked separately, with its own message.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review, focusing on the final exclusion validation updates.
- Rejected ineffective file exclusions: Added filesystem validation so an existing file cannot be configured where the scanner can only prune directories, while missing directory paths remain valid for shared schemas.
- Guarded default-excluded targets: Added an explicit error when a positional target is covered by an always-skipped directory such as
node_modules. - Updated guidance and coverage: Documented the directory-only contract and added focused scanner and command regressions.
azure/gpt-5.6-sol ｜ 𝕏
…nd (#4) Expands docs/ from 2 pages to 10 (install, services, worktrees, sharing, varlock, agents, commands, troubleshooting; index and getting-started rewritten; sidebar grouped) and fixes the six behaviours the sweep uncovered: - child exit codes are propagated instead of being overwritten with 1 by failJsonAware - the sail:up host-port preflight is scoped to the services compose declares - the service shells' hint puts ace flags before the `--` separator - sail:share --json prints exactly one JSON document, and its URL scanner detaches from the child's streams once the tunnel URL is known - sail:up and sail:sync-env report a skipped encrypted .env.local with the same sentence - sail:sync-env exits 1 for that skip under --json too, not only in human mode Feature: sail:install declares @auditExtraPatterns root decorators in .env.schema for the Adonis env.get('KEY') and KEY: Env.schema.… idioms (varlock dmno-dev/varlock#1075), append-only and divider-aware. Also corrects the documented git floor to 2.36.
2 participants
Follow-up to the escape-hatch idea in #1074: built-ins for conventional idioms, user config for everything else.
What
A root decorator for project-specific code-scan patterns, for env access the built-in patterns can't see (NestJS
configService.get('KEY'), or anything else behind a wrapper):fileTypes=[tf, yaml]on a call restricts that call's patterns to those file types. Because the scan only reads files whose extension it recognizes, naming an unrecognized one is also what brings those files into the walk, so this is how a pattern reaches Terraform, Helm values, CI configs or shell scripts at all.@auditIgnorePaths.Files brought in this way have no known language: the built-in patterns never apply to them, and with no comment syntax to go on their raw contents are scanned.
Directory exclusions
@auditIgnorePaths()and--ignorefollowed neither of the two things their docs implied: a bare name matched at any depth, and a multi-segment entry like the documentedgenerated/configmatched nothing at all, silently. An entry now states which kind of match it wants:@import():./or../relative to the scanned directory,~/for home, or absoluteThe mode is never inferred from whether a separator happens to be present, so
docsand./apps/docscan't be confused for each other. Two mistakes that could only ever match nothing are now errors naming the fix, instead of silently excluding nothing: a path without one of those prefixes, and a path resolving outside the scanned tree. Docs note that absolute and~/paths are machine-specific and./is the right choice in a committed schema.Normalization moved into the scanner so the decorator and the CLI flag share one implementation. Previously
--ignoreskipped it entirely, so--ignore ./vendor/matched nothing while@auditIgnorePaths(./vendor/)worked.Scanner fixes this surfaced
config.get('app.database.url')could never match. Custom patterns now run over content with comments masked and strings intact.//and/* */but only stepped past them, so a commented-out`${/* process.env.KEY */ ...}`was reported as a live reference. This was already the case for the built-in patterns, independent of this PR.How
decorators.ts: one-line registration next toauditIgnorePaths.audit.command.ts: reads the patterns and each call'sfileTypes, rejects unknown options and empty lists.env-var-scanner.ts:customsyntax variant, per-pattern extension scope feeding both file discovery and matching, and the masking changes above.@auditExtraPatterns()reference entry, a corrected@auditIgnorePaths()entry, and avarlock auditpage that now documents what the scan actually reads (extensions, size cap, always-skipped directories) and links all three audit decorators. VS Code intellisense entries included.Verification
Scanner-level and command-level specs, plus a parser-to-command integration spec that starts from real
.env.schematext so the documented syntax is proven to survive parsing. Coverage includes the extension scoping in both directions, built-ins never applying to widened files, regressions for both masking fixes, and each exclusion form (bare name at depth,./path,~/expansion, absolute path inside the root, outside-the-root rejection, trailing and Windows separators).