feat(azure): Azure Marketplace solution-template package - #307
Conversation
…loy/azure) Free "Get It Now" Azure Application offer: an ARM template + portal wizard that deploy one Ubuntu 24.04 LTS VM running the ghcr image behind a Caddy reverse proxy with automatic HTTPS (Let's Encrypt, HTTP-01 pinned) and a restriction-preserving TLS fallback (plain HTTP only when the customer left 443 open to the internet; self-signed on 443 otherwise, failing closed on missing arguments). scripts/build-azure-package.mjs produces the two-file Partner Center zip: resolves the app (ghcr) and Caddy (Docker Hub) tags to manifest digests, embeds the installer base64 into the template, and gates on apiVersion age (warn >=540 days, fail >=700; policy rejects at 730). The dispatch workflow validates the package with a tag+sha256-pinned arm-ttk before uploading it. Verified: arm-ttk Test-AzMarketplacePackage 35/35, shellcheck clean, 32 unit tests including a hermetic end-to-end CLI run against a local registry stub. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const parsed = parseImageRef(ref); | ||
| const endpoints = registryEndpoints(parsed); | ||
|
|
||
| const tokenResponse = await fetchImpl(endpoints.token); |
There was a problem hiding this comment.
The dataflow is real, but it is not a vulnerability. The "file data" is this repo's own tracked package.json, read by a build script that only maintainers and CI run — and anyone who can edit that file already has code execution through npm scripts. The registry host is not file-derived: it comes from the hardcoded APP_IMAGE_REPO / CADDY_IMAGE_REF constants, so no request can be steered to a different host.
ee16161 still tightens the one part that is not a constant. parseImageRef now validates the tag against the OCI tag grammar (^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$), so a version string carrying a path segment fails the build instead of travelling into a URL path. If the alert survives that, it should be dismissed as a false positive rather than worked around further.
| } | ||
|
|
||
| const manifestUrl = `${endpoints.manifestHost}/v2/${parsed.repository}/manifests/${parsed.tag}`; | ||
| const response = await fetchImpl(manifestUrl, { |
There was a problem hiding this comment.
The dataflow is real, but it is not a vulnerability. The "file data" is this repo's own tracked package.json, read by a build script that only maintainers and CI run — and anyone who can edit that file already has code execution through npm scripts. The registry host is not file-derived: it comes from the hardcoded APP_IMAGE_REPO / CADDY_IMAGE_REF constants, so no request can be steered to a different host.
ee16161 still tightens the one part that is not a constant. parseImageRef now validates the tag against the OCI tag grammar (^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$), so a version string carrying a path segment fails the build instead of travelling into a URL path. If the alert survives that, it should be dismissed as a false positive rather than worked around further.
There was a problem hiding this comment.
Pull request overview
Adds the Azure Marketplace solution-template package for deploying LibreDB Studio on an Azure VM.
Changes:
- Adds ARM deployment, portal wizard, and first-boot installer.
- Adds package building, validation workflow, and tests.
- Adds Marketplace listing content and operational documentation.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/azure-marketplace-package.yml |
Builds, validates, and uploads the package. |
scripts/build-azure-package.mjs |
Pins images and creates the Partner Center zip. |
tests/unit/build-azure-package.test.ts |
Tests package generation and validation. |
deploy/azure/src/mainTemplate.json |
Defines Azure infrastructure and outputs. |
deploy/azure/src/createUiDefinition.json |
Defines the deployment wizard. |
deploy/azure/src/install.sh |
Installs and configures the application. |
deploy/azure/package-version.txt |
Tracks the Marketplace package version. |
deploy/azure/README.md |
Documents build and publishing procedures. |
deploy/azure/AZURE_MARKETPLACE_PLAN.md |
Provides the Marketplace implementation plan. |
deploy/azure/listing/listing-fields.md |
Supplies Partner Center listing fields. |
deploy/azure/listing/description.html |
Supplies the Marketplace description. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| printf 'AUTH_BOOTSTRAP=off\n' | ||
| printf 'JWT_SECRET=%s\n' "$JWT_SECRET" | ||
| printf 'NEXT_PUBLIC_AUTH_PROVIDER=local\n' | ||
| printf 'ADMIN_EMAIL=%s\n' "$APP_ADMIN_EMAIL" | ||
| printf 'ADMIN_PASSWORD=%s\n' "$APP_ADMIN_PASSWORD" |
There was a problem hiding this comment.
Fixed in ee16161. Verified the whole chain before changing anything: Dockerfile:42 sets NODE_ENV=production, src/lib/auth.ts:131-146 returns true for any non-loopback host when no override is set, and Caddy forwards the original Host — so the cookie was marked Secure over an http:// origin and the browser dropped it. docs/DISTRIBUTION.md:867 already ships AUTH_COOKIE_SECURE=false for the LAN channel for exactly this reason, which makes this the second time the trap bites.
The :80 deployment now writes the override into /etc/libredb-studio.env.
The second half of your point — update and restart the app when entering the HTTP fallback — is resolved differently: the HTTP fallback is gone. The Caddyfile now chains issuer acme → issuer internal, so a failed issuance stays on :443 with a self-signed certificate. The browser still speaks https there, so the Secure cookie is accepted and no app-level reconfiguration is needed on that path at all. That also removes the coupling that produced this bug: a rarely-exercised bash branch had to know about the app's cookie policy.
| "outputs": { | ||
| "applicationUrl": { | ||
| "type": "string", | ||
| "value": "[if(parameters('enableHttps'), concat('https://', reference(resourceId('Microsoft.Network/publicIPAddresses', variables('publicIpName'))).dnsSettings.fqdn), concat('http://', reference(resourceId('Microsoft.Network/publicIPAddresses', variables('publicIpName'))).ipAddress))]" |
There was a problem hiding this comment.
Fixed in ee16161 — structurally, rather than by surfacing a runtime URL.
ARM outputs are computed from parameters at deploy time, so a URL that depends on what the installer decided at runtime is not representable in a solution template without deployment-script plumbing. The fix is therefore to stop making the URL depend on a runtime decision: the app never moves off :443 now (issuer acme → issuer internal), so https://<fqdn> is the live URL by construction, whether the certificate ended up trusted or self-signed.
| }, | ||
| "notes": { | ||
| "type": "string", | ||
| "value": "[if(parameters('enableHttps'), if(equals(parameters('appSourceAddressPrefix'), 'Internet'), concat('If the TLS certificate could not be issued, the installer serves the application over plain HTTP at http://', reference(resourceId('Microsoft.Network/publicIPAddresses', variables('publicIpName'))).ipAddress, ' and records the reason in /etc/libredb-studio.info on the virtual machine.'), 'If the TLS certificate could not be issued, the installer keeps the application on port 443 with a self-signed certificate so that your source restriction is never widened; your browser will warn you. The reason is recorded in /etc/libredb-studio.info on the virtual machine.'), 'The application is served over plain HTTP. Put a TLS terminating gateway in front of it before exposing it beyond a trusted network.')]" |
There was a problem hiding this comment.
Confirmed and fixed in ee16161. The wizard regex does accept 0.0.0.0/0 (createUiDefinition.json:163), the installer treated it as unrestricted, and this output did not — so Azure would have described the opposite of what the VM actually did.
One addition your comment did not cover: the InfoBox at createUiDefinition.json:173 carried the same claim, and unconditionally, so it was wrong for 0.0.0.0/0 too.
Rather than teaching all three places about both spellings, the branch they were describing is gone. The installer no longer inspects the source range at all and the fifth argument was dropped from commandToExecute. A new cross-file test ties the argument count the template passes to the count the installer reads, so a future argument change cannot drift silently again.
| const outputs = Object.keys(ui.parameters.outputs).sort(); | ||
| const params = Object.keys(template.parameters).sort(); | ||
| expect(outputs).toEqual(params); |
There was a problem hiding this comment.
Correct — the assertion compared sorted key names only, and the test name promised more than that. Fixed in ee16161:
- an explicit
name -> ARM typetable now pins every parameter, so your exact example (enableHttpsdrifting frombooltostring) fails the test; - a second test checks that the controls behind the non-string parameters really emit that type: the
enableHttpsOptionsGroup'sallowedValuesare booleans, and the disk slider'sdefaultValue/min/maxare integers; - the key-equality test keeps its narrower name.
| > **Repo kuralı:** Bu iş `deploy/azure/` altında yaşar. `deploy/gcp/` (Google Cloud) ve | ||
| > `deploy/rancher/` (SUSE) ayrı çalışmalardır, karıştırılmamalıdır. | ||
|
|
||
| ### 5.3 `deploy/azure/src/install.sh` |
There was a problem hiding this comment.
All three claims verified against the file, and fixed in ee16161. For the record, since the third one is the serious one:
| plan copy | shipped src/install.sh |
|
|---|---|---|
| missing web-source argument | ="Internet" — fail-open |
="restricted" — fail-closed |
| missing site address | =":80" silently |
exit 1 |
| credentials | unquoted <<EOF heredoc |
printf '%s' |
The unquoted heredoc means a $(...) sequence inside the customer's password would have been expanded by the shell, as root. The copy was also missing the character validation, the 0600 log mode and the empty-JWT_SECRET check.
Sections 5.3–5.5 now point at the sources instead of duplicating them (−880 lines). The rationale prose that is not derivable from the code stays where it was, since that is the part a source link cannot replace.
…eak plain-HTTP login Addresses the PR review findings, blocking one first. A plain-HTTP deployment could never log in. The image runs with NODE_ENV=production, where shouldMarkCookieSecure() marks the auth cookie Secure for every non-loopback host, and a browser on http:// discards such a cookie - so the sign-in silently looped back to the login page. The ":80" deployment now writes AUTH_COOKIE_SECURE=false, the same override docs/DISTRIBUTION.md already ships for the LAN channel. Reachable two ways before this: enableHttps=No, and the certificate fallback below. The TLS fallback moves out of bash and into Caddy. The site declares `issuer acme` and then `issuer internal`, which the Caddyfile adapter keeps in that order (verified against the pinned caddy:2-alpine: `Valid configuration`, issuers [acme(tls-alpn disabled), internal]). That deletes the entire branch that rewrote the Caddyfile after the health gate - the .https/.fallback copies, the restart, the restore hint, and the fifth installer argument - and with it three findings whose single root cause was keeping one decision in sync by hand across bash, the ARM outputs and the wizard text: - applicationUrl advertised https://<fqdn> while the fallback had moved the app to plain HTTP on :80, leaving no :443 listener at all. - the `notes` output and the wizard InfoBox treated only `Internet` as unrestricted while the installer also accepted `0.0.0.0/0`, so Azure described the opposite of what the VM had done. The health gate now only reports: it waits for any certificate, then re-probes without -k to establish which one arrived. A failed issuance is no longer a degradation, so /etc/libredb-studio.info carries no restore procedure - internal certificates are short-lived, so every renewal cycle retries ACME and the deployment upgrades itself. Also: - the source-agreement test compared key names while its name claimed "compatible kind". It now pins each parameter's ARM type and checks that the controls behind the non-string ones really emit that type. A new cross-file test ties the argument count the template passes to the count the installer reads. - parseImageRef validates the tag against the OCI grammar. The tag is the only part of the app ref that is not a constant - it comes from package.json or --version - and it is interpolated into the registry URLs. - the registry test double splits on the full origin instead of a URL substring. - the plan's verbatim copies of install.sh, mainTemplate.json and createUiDefinition.json are replaced by pointers to the sources. The copy had already drifted: it defaulted a missing argument to the unsafe side and wrote the customer's password through an unquoted heredoc, so a `$(...)` sequence inside a password would have expanded as root. Section 6.2's fallback and recovery tests are rewritten around self-healing, and the open ZeroSSL question is closed - `issuer zerossl` requires a paid API key, which is exactly why the redundancy comes from `issuer internal`.
Review round: 8 findings triaged, 6 valid, all addressed (ee16161)One was blocking — a plain-HTTP deployment could never log in. The image runs with The design change behind three of the fixesThree findings — a wrong
That deleted the entire bash fallback machine: the
The installer's health gate now only reports: it waits for any certificate, then re-probes without This also closes the plan's open ZeroSSL question. The two CodeQL alerts on
|
AZURE_MARKETPLACE_PLAN.md was working material for building the channel, not something the shipped package needs, so it leaves the repo entirely (-1301 lines). Everything that still had a job moves or goes: - the acceptance criteria for a real deployment were the only content the README linked out for, so they are now in the README itself - including the two the last commit added: log in and RELOAD (a Secure cookie on plain HTTP fails no health probe), and the fallback check that a certificate failure leaves the app on the same port and heals itself. - the "§5.6" / "§5.7" / "§7.3" cross-references in the builder, its tests, the workflow and the listing fields are gone; nothing points at a missing file. Removing the file also settles two open items on its own: the duplicated, already-drifted copies of install.sh and the templates cannot rot any further, and the repo no longer carries a Turkish document in a public tree.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
deploy/azure/src/install.sh:96
- On a CustomScript rerun this preserves the entire old environment, not only
JWT_SECRET. Changes to protected settings (for example credentials orenableHttps) trigger reruns, so new admin credentials are ignored and switching HTTPS modes leavesAUTH_COOKIE_SECUREstale—HTTPS→HTTP recreates the login failure this PR addresses. Preserve the existing JWT secret, but atomically rewrite the other settings from the current arguments and restart the app.
# Written once and never rewritten: if the extension re-runs (VM reimage, extension
# update), regenerating JWT_SECRET would invalidate every existing session.
if [ ! -f /etc/libredb-studio.env ]; then
deploy/azure/src/createUiDefinition.json:126
defaultValuemust match anallowedValues[].value, but these values are booleans and"Yes"is only a label. The portal therefore has no valid default for this required control instead of defaulting HTTPS on. Use the boolean value.
"defaultValue": "Yes",
deploy/azure/src/install.sh:226
enable --nowstarts inactive units but does not restart units that are already running. On an extension/package rerun, the newly written image refs and Caddyfile therefore remain inactive, and the health gate can pass against the old containers. Explicitly restart both units after reloading systemd.
systemctl daemon-reload
systemctl enable --now libredb-studio libredb-caddy
deploy/azure/src/mainTemplate.json:163
- These arrays contain only allow rules, so Azure's built-in priority-65000
AllowVNetInBoundrule still permits every port from the local/peered VNet. Consequently a CIDR web restriction is not exclusive, and leaving SSH blank does not “block inbound SSH completely” as the wizard claims. Add higher-priority deny rules after the intended web/SSH allows (and an explicit Bastion allow if desired), or clearly scope the UI promises to public-Internet ingress.
"securityRules": "[if(empty(parameters('sshSourceAddressPrefix')), variables('webRules'), concat(variables('webRules'), variables('sshRules')))]",
deploy/azure/README.md:60
- This fallback test cannot be staged as written. The template defines
securityRulesinline, so redeploying it replaces the NSG rule array and removes a manually addedDenyAcme; on a fresh deployment the NSG does not exist yet to receive the rule. Use a test-only template variant containing the deny rule or an upstream firewall that the deployment does not overwrite.
- **TLS fallback.** Add a `DenyAcme` NSG rule blocking port 80, then deploy with a
**fresh** `dnsLabelPrefix`. Expected: deployment still `Succeeded`; `curl -fsSk
deploy/azure/README.md:65
- Restarting Caddy does not replace a still-valid internal certificate. With the documented 12-hour lifetime, renewal normally begins only after roughly eight hours, so this acceptance step will not make a trusted certificate “arrive” immediately. Either wait for the renewal window or explicitly force fresh issuance in the disposable test procedure.
restore procedure. Then delete the NSG rule and `systemctl restart libredb-caddy` — a
trusted certificate arrives on its own, with no manual step.
| install -d -m 0755 /opt/libredb /opt/libredb/data /opt/libredb/caddy \ | ||
| /opt/libredb/caddy/data /opt/libredb/caddy/config |
There was a problem hiding this comment.
Valid, and fixed in bd61b99. Both premises check out:
DatabaseConnectioncarries a plaintextpasswordandconnectionString(src/lib/types.ts:48,60,62) and the storage layer persistsconnectionsverbatim, so the SQLite store really does hold the customer's database credentials.- The umask claim is measurable, so I measured it rather than reasoning about it: a file created inside that bind mount by the container lands at
0644. With the parent at0755, any local account on the VM could read it./etc/libredb-studio.envalready met the0600bar, so the store holding more credentials than the env file was the weaker link.
The directory mode is the right control point — nothing in the installer governs the umask of files the containers create later — so both secret-bearing directories are now 0700, and the shared parents stay 0755.
One disagreement: keeping all the Caddy directories at 0755 is wrong. /opt/libredb/caddy/data is where certmagic writes the TLS private keys and the ACME account key, so it moves with the app's data directory rather than relying on certmagic tightening its own subdirectories. /opt/libredb/caddy and /opt/libredb/caddy/config stay 0755.
Verified rather than assumed, since getting this wrong fails the deployment outright:
- the app container starts as root, chowns its data dir to
nextjsand drops privileges via gosu (docker-entrypoint.sh:11-16), andchowndoes not touch the mode; docker run --rm caddy:2-alpine id→uid=0(root), so a root-owned0700/datais fine;- a
0700directory chowned to uid 1001 is fully writable by a process running as uid 1001 (checked directly); install -d -m 0700also repairs a directory an earlier version left at0755(checked:755→700), so a re-run on an existing VM fixes it.
A stat check on both directories is now part of the acceptance criteria in deploy/azure/README.md, and a unit test pins the modes in install.sh so the two secret directories cannot drift back into the 0755 line.
…traversable /opt/libredb/data holds the SQLite store, and DatabaseConnection records carry a plaintext `password` and `connectionString` (src/lib/types.ts). The directory was created 0755, and the file inside is created with the container's umask - measured as 0644 - so every local account on the VM could read the customer's database credentials. /etc/libredb-studio.env already met the 0600 bar; the store that holds more credentials than it did not. The directory mode is the control point, since nothing here controls the umask of files the containers create later. Both directories that hold secrets are now 0700: - /opt/libredb/data - connection records with plaintext credentials - /opt/libredb/caddy/data - the TLS private keys and the ACME account key The review note suggested keeping the Caddy directories at 0755. That is right for /opt/libredb/caddy and its config, and wrong for its data directory, which is where certmagic writes private keys - so it moves with the other one rather than relying on certmagic tightening its own subdirectories. Neither container needs a change, verified rather than assumed: the app container starts as root, chowns its data dir to `nextjs` and drops privileges via gosu (docker-entrypoint.sh), the Caddy image runs as root (uid 0), and `chown` does not touch the mode - a 0700 directory chowned to uid 1001 is fully writable by uid 1001. A re-run also repairs a directory left at 0755 by an earlier version, because `install -d` applies the mode to directories that already exist.
|
What
Adds the technical package for a free ("Get It Now") Azure Marketplace listing of LibreDB Studio — Azure Application offer, solution-template plan — under
deploy/azure/.deploy/azure/src/mainTemplate.json— ARM template: VNet + NSG + static public IP (DNS label) + Ubuntu 24.04 LTS VM (Canonical marketplace image, implicit managed disk) + CustomScript extension viaprotectedSettings.deploy/azure/src/createUiDefinition.json— portal wizard (credentials, size selector, HTTPS toggle, source-range restrictions with strict IPv4 validation).deploy/azure/src/install.sh— first-boot installer: Docker from the Ubuntu archive, digest-pinned app + Caddy containers, automatic HTTPS (Let's Encrypt over HTTP-01, TLS-ALPN disabled because port 443 may be restricted), and Caddy's own issuer chain as the fallback —issuer acme→issuer internal, so a failed issuance keeps the app on the same port with a self-signed certificate, never widens the customer's source restriction, and heals itself on the next renewal cycle. Health gates fail the deployment loudly if nothing is served at all.scripts/build-azure-package.mjs(+ 38 unit tests) — builds the two-file Partner Center zip: resolves ghcr/Docker Hub tags to manifest digests, validates the tag against the OCI grammar, embeds the installer, gates on apiVersion age (warn ≥540 d, fail ≥700 d; policy rejects at 730 d)..github/workflows/azure-marketplace-package.yml— dispatch workflow: build → arm-ttk marketplace validation (release pinned by tag and sha256) → artifact upload.deploy/azure/listing/— Partner Center listing texts, with the character limits enforced by the unit tests.deploy/azure/README.md— build, validate, and the acceptance criteria for a real deployment.Verification
Test-AzMarketplacePackage: 35 tests, 0 failed (1 benign warning:applicationUrlconcat).caddy:2-alpine:Valid configuration, andcaddy adaptconfirms the automation policy carries[acme(tls-alpn disabled), internal]in that order.shellcheckclean;bun run format/lint/typecheck/knip/test/buildall clean; 38/38 unit tests including a hermetic end-to-end CLI run against a local registry stub.Microsoft.Network/* 2025-07-01,Microsoft.Compute/* 2026-03-01(both latest GA).Out of scope
Partner Center account + offer creation, a real
az deployment group createagainst a live subscription (needs one), listing media (logo 300×300, 1280×720 screenshots — brand decision), and thedistribution/channels.yamlentry, which lands after Go live.The working plan document used to build this package is deliberately not in the repo: it was preparation material, and everything the package needs to be built, validated and shipped is in
deploy/azure/README.md.🤖 Generated with Claude Code