Repository navigation
fix(lint): surface unloadable media variable defaults, stop reading data-var-src ids as paths - #3406
Merged
Merged
Conversation
…ata-var-src ids as paths A data-var-src value the runtime refuses to load is dropped at bind time and the element's authored fallback src renders instead, so the video ships the wrong media and the render still exits 0. lint said nothing, because the scheme allowlist only existed inside the runtime. Move that predicate into @hyperframes/parsers, where both the runtime and the linter can reach it, and error at lint time on any declared default it rejects. The value provably cannot load, so there is no false positive. While reproducing that, lint turned out to report an unrelated missing file: `\bsrc\s*=` also matches the tail of `data-var-src="bg"` (hyphen to `s` is a word boundary), and `[^>]*` is greedy, so the variable id beat the real src earlier in the same tag. Every binding was reported as a missing asset named after the variable, and `<audio data-var-src>` was told the render would be silent. All three copies of that regex now share one helper that requires whitespace before the attribute.
miguel-heygen
force-pushed
the
fix/var-src-unsafe-url-lint
branch
from
August 21, 2026 23:01
8004aab to
ec52f49
Compare
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
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.
What
lintnow errors when a composition variable's default is a media URL the runtime willrefuse to load, and it no longer mistakes a
data-var-srcvariable id for a file path.Closes #3368.
Why
A
data-var-srcvalue outside the runtime's scheme allowlist is dropped at bind time andthe element's authored fallback
srcrenders instead. The video ships the wrong media andthe render exits
0. The rejection was aconsole.warninside a render worker, which in abatch is invisible.
lintcould not catch it because the allowlist only existed insideapplyVariableBindings.ts. This moves that predicate into@hyperframes/parsers, wherethe runtime and the linter both reach the same one, and errors on any declared default it
rejects. The value provably cannot load, so the rule has no false-positive case.
Rejecting
file:stays correct — the allowlist is a security boundary and is unchanged.The bug was the silence, not the policy.
A second defect found while reproducing it
Linting the reporter's repro produced an error about a missing file named
bg, which isthe variable id, not a file anyone referenced:
Three rules shared this regex:
\bsrc\s*=also matches the tail ofdata-var-src="bg"— hyphen tosis a word boundary— and
[^>]*is greedy, so the variable id beat the realsrcearlier in the same tag.Every element using a variable binding was reported as referencing a missing asset named
after its variable, and
<audio data-var-src>was additionally told the render would besilent. All three now share one helper that requires whitespace before the attribute.
How
isSafeMediaUrlmoves frompackages/core/src/runtime/applyVariableBindings.tstopackages/parsers/src/compositionVariables.ts; the runtime imports it rather than owninga copy, so lint and the runtime cannot drift.
unloadable_media_variable_defaulterror inpackages/lint/src/rules/composition.ts,applied to
image-typed variables and to any id an element binds withdata-var-src.mediaSrcTagRe()inpackages/lint/src/utils.tsreplaces the three copies of the srcregex in
project.ts(×2) andhevcPreviewLint.ts.Test plan
Reporter's repro, an
<img>with a red fallbacksrcbound to a variable defaulting to anabsolute
file://path to a blue image.Before (
[email protected]) — reports a file that was never referenced, says nothingabout the binding:
After (this branch) — the phantom file is gone, the real defect is named:
Suites, each with its own package config:
packages/corepackages/parserspackages/lintSix new tests: four on the new rule (
file://on an image variable,file://on anon-image variable a
data-var-srcconsumes, relative/http/data:imagestaying quiet, anunbound scalar not being treated as a URL) and two on the regex (a real
srcreportedinstead of the variable id, no invented asset when the src is a templating token).
Not covered
--variables/--variables-filevalues are still only checked at bind time. Lint seesdeclared defaults, not run-time overrides, so passing a rejected URL through the flag
keeps the original silent-fallback behaviour. Fixing that belongs at the CLI boundary and
is a separate change.
framework's existing policy for media that cannot load (a missing
srcrenders a brokenimage and exits
0,frameCapture.ts:387); changing it is a policy decision, not a bugfix, and would need its own discussion.