web-app-serve: helm wrapper chart + harden apply-config - #91
Open
thenav56 wants to merge 4 commits into
Open
Conversation
Migrate the web-app-serve publish workflow to also publish a Helm wrapper chart (docker image path already integrated upstream): - Rename publish-web-app-serve.yml -> helm-publish.yaml. - Bump toggle-corp/web-app-serve-action v0.1.1 -> v0.2.0 (helm-capable) and set helm_publish: true. - Grant contents: read and pull-requests: read (the provenance writer resolves the PR from the commit); keep packages: write. - Add web-app-serve/helm wrapper chart depending on web-app-serve-helm 0.1.1 (aliased app), with SET-BY-CICD image placeholders and pre-seeded ingress.annotations. Temp branch trigger (feat/web-app-serve-helm) added to exercise the workflow; remove after merge.
…ed placeholders resolve to JS undefined + warn)
The custom ^VITE_ apply-config had no hardening: no sed metacharacter
escaping, and a ${VALUE:-undefined} default that only affected set-but-empty
vars (injecting the truthy string "undefined" into quoted JS defines) while
leaving omitted vars leaking the literal placeholder marker.
Rewrite it to mirror the helix-client reference (adapted to VITE_):
- Escape sed replacement metachars (\, &, |) so URLs/tokens substitute
literally instead of corrupting output or exiting the container.
- Drop the :-undefined quirk; set-but-empty now substitutes to an empty value.
- After the substitution loop, warn (stderr) about every leftover
WEB_APP_SERVE_PLACEHOLDER__VITE_* marker, then rewrite the quoted-JS markers
to bare 'undefined' (falsy) so an omitted optional var no longer leaks a
truthy marker into the bundle.
overrideDefineForWebAppServe emits an UNQUOTED marker for the one
Schema.boolean key (VITE_ALLOW_UNVERIFIED_USERS), which the quoted-JS rewrite
cannot blank; bake it as an overridable default ENV in the Dockerfile final
stage so the substitution loop always fills it (avoids a bare-identifier
ReferenceError on an unset runtime).
Verified: validator (VITE_) passes; docker bare-run leak check reports 0
markers; &/| substitute literally; unset optional vars resolve to undefined
with a stderr warning.
tnagorra
reviewed
Jul 20, 2026
| # `WEB_APP_SERVE_PLACEHOLDER__VITE_ALLOW_UNVERIFIED_USERS` identifier (ReferenceError). | ||
| # Baking a default here means the substitution loop always fills it; deployments | ||
| # can still override it at runtime. | ||
| ENV VITE_ALLOW_UNVERIFIED_USERS=true |
Contributor
There was a problem hiding this comment.
This should be "false" by default?
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
2 participants
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.
Changes
ci:— publish helm wrapper chart viaweb-app-serve-action@v0.2.0The web-app-serve Docker image path is already integrated upstream; this adds
the Helm wrapper chart so CI publishes both the image and a deployable chart.
publish-web-app-serve.yml→helm-publish.yaml; action bumpedv0.1.1→v0.2.0(helm-capable,helm_publish: true); permissions widened withcontents: read+pull-requests: read(the provenance writer resolves the PR from the commit) alongside the existingpackages: write. Real trigger branches (develop,project/*) kept.web-app-serve/helm/(mapswipe-web-helm), depending onoci://ghcr.io/toggle-corp/web-app-serve-helm:0.2.1aliasedapp. Minimalvalues.yamlnests everything underapp:withSET-BY-CICDimage placeholders and a pre-seededingress.annotations: {}. CI stamps the image name/tag + provenance annotations and pushes the chart to GHCR on every push.web-app-serve/helm/.gitignoreexcludes thehelm dependency updateartifacts (charts/,Chart.lock); CI regenerates them.fix:— harden the custom^VITE_apply-configThe runtime apply-config (
web-app-serve/web-app-apply-config.sh) had nohardening. It is now brought in line with the current convention:
\,&,|) so URLs/tokens with them substitute literally instead of corrupting output or exiting the container.${VALUE:-undefined}quirk (it injected the truthy string"undefined"into quoted JS defines only for set-but-empty vars); set-but-empty now substitutes to an empty value.WEB_APP_SERVE_PLACEHOLDER__VITE_*marker, then rewrite the quoted-JS markers to bareundefined(falsy) so an omitted optional var no longer leaks a truthy marker into the bundle.VITE_ALLOW_UNVERIFIED_USERS=trueas an overridable default ENV in the Dockerfile final stage:overrideDefineForWebAppServeemits an unquoted marker for the oneSchema.booleankey, which the quoted-JS rewrite cannot blank — without a baked default an unset runtime would leave a bare-identifierReferenceError. Deployments can still override it. (Preserves the value the build stage already baked; flip tofalseat deploy time if unverified users should be disallowed.)Verification
helm dependency update+helm lint+helm template(with the deploy-time--set app.ingress.hostname/ingressClassName/image.nameflags) all pass.VITE_): 7/7 pass.&/|values substitute literally; unset optional vars resolve toundefinedwith a stderr warning;VITE_ALLOW_UNVERIFIED_USERSsubstitutes to a baretrue.TODO before/after merge
feat/web-app-serve-helmentry fromon.push.branchesin.github/workflows/helm-publish.yaml— it exists only to test the publish workflow end-to-end on this branch (the slash in the branch name publishes toghcr.io/mapswipe/mapswipe-web-dev/mapswipe-web-dev-helm).