Skip to content

Let a Phala target name the public URL it serves - #46

Merged
2xburnt merged 5 commits into
mainfrom
work/burntbot/phala-public-url-20260926T002958Z
Sep 26, 2026
Merged

2xburnt merged 5 commits into
mainfrom
work/burntbot/phala-public-url-20260926T002958Z

Conversation

@2xburnt

@2xburnt 2xburnt commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

A Phala target may now declare publicUrl, a bare https:// origin. When it is set, the health check and the returned deployment-url use it instead of the URL Phala reports.

Why: TeeLS production (teels-prod) terminates TLS inside the CVM with dstack-ingress in passthrough mode, so the gateway-terminated URL Phala reports (<app-id>-443.dstack-pha-prod9.phala.network) never answers; only teels.burnt.com (and the -443s passthrough host) do. Without this, the first CI deploy of TeeLS would redeploy the CVM and then fail its health check.

  • scripts/policy.mjs: validate targets.<role>.publicUrl (https, origin only); bundle rebuilt.
  • phala-deploy.yml: the URL step short-circuits to publicUrl when set; otherwise unchanged.
  • Tests for both; AGENTS.md documents the field.
  • phala-deploy.yml "Deploy CVM": only the candidate CVM is created when no CVM has the target's name. A release miss fails with ::error:: and deploys nothing; release is --cvm-id update only. Unconditional, not a policy knob. Why: a release CVM's DNS and verifier allowlists point at its app id and measurements, so one deleted or renamed after a caller's own checks (teels-internal#109's guard) would otherwise come back as a new instance and the run would pass. Test runs the real step against a stubbed CLI and asserts the release path never passes -n; it fails on the old step.
  • Consumer impact: tls-app-attest has never run a release deploy (staging only) and pins v1.6.0; its satya-tee-attestation-production CVM has to be provisioned by hand before its first release on v1.7.0.
  • Pins advanced to # v1.7.0 (policy-tool refs at the implementation commit, uses: at the pin advance); cut v1.7.0 after merge so consumers can resolve it.

Checks: pnpm run lint, node --test (87/87), actionlint.

Part of DO-513

A service that terminates TLS inside the CVM (dstack-ingress with TLS
passthrough) does not answer on the gateway-terminated URL Phala reports,
so the health check fails even when the deploy succeeded. A target can
now declare publicUrl, a bare https origin, and the health check and the
deployment-url output use it.
Copilot AI lite review requested due to automatic review settings September 26, 2026 00:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Address the validator edge cases and policy-tool pin, then update the output description.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 Medium severity · 1 Low severity

Open (4)
What changed in this PR

Adds optional Phala target publicUrl support for TLS passthrough deployments, using it for health checks and deployment output.

Changes:

  • Validates and bundles HTTPS-origin configuration.
  • Resolves deployment URLs from publicUrl when configured.
  • Adds tests and documentation.
File Summary Review findings
tests/​workflows.test.mjs Tests workflow URL handling. No final findings.
tests/​policy.test.mjs Tests publicUrl validation. No final findings.
scripts/​policy.mjs Validates configured public URLs. Moderate: reject trailing empty ? or #; advance workflow policy-tool pins.
scripts/​policy.bundle.mjs Rebuilt validator bundle. Moderate: rebuild after fixing trailing delimiter validation.
AGENTS.md Documents publicUrl. No final findings.
.github/​workflows/​phala-deploy.yml Uses configured public URLs for health checks and outputs. Moderate: advance the hard-coded policy-tool ref. Nit: update the deployment-url output description.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/phala-deploy.yml
Comment thread scripts/policy.bundle.mjs Outdated
Comment thread scripts/policy.mjs Outdated
Comment thread .github/workflows/phala-deploy.yml
URL parsing normalizes away an empty `?` or `#`, surrounding whitespace,
and embedded tabs and newlines, so a value like `https://host?` parsed to
a bare origin and passed. The workflow uses the raw string and appends the
health path, which would have requested `https://host?/health`. The
validator now requires the raw value to equal the parsed origin, with or
without one trailing slash.

The deployment-url output described the URL Phala reports; with publicUrl
set it is the policy's endpoint, so it now says it is the endpoint that
passed the health check.
The workflows check the policy scripts out by SHA, so until these move a
consumer runs the v1.6.0 validator: publicUrl would go unvalidated into
the health check and deployment-url. The flow callers move next, to this
commit.
A caller pinned at the implementation commit would still resolve the
policy scripts from v1.6.0. The callers name the pin advance; the
policy-tool refs name where the scripts landed.
Copilot AI review requested due to automatic review settings September 26, 2026 00:41
@2xburnt

2xburnt commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Address the validator edge cases and policy-tool pin, then update the output description.

All four are fixed and their threads resolved. d7e4d93 checks publicUrl as written against its parsed origin and updates the output description. 3332fc8 and 081e88f advance the pins to # v1.7.0, the tag to cut once this merges.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (4)

The deploy looked the target's CVM up by name and created one on a miss,
for either role. A release CVM's identity lives outside this flow: DNS
names its app id and relying-party allowlists name its measurements. A
release CVM deleted or renamed after a caller's own checks came back as a
new instance nothing points at, and the run reported success.

Only the candidate is created on a miss now. A release deploy that finds
no CVM of that name fails; the release CVM is provisioned by hand once.
The rule is unconditional rather than a policy knob.
Copilot AI review requested due to automatic review settings September 26, 2026 02:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Add an execution test covering the PUBLIC_URL branch and its deployment output.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Add execution test for PUBLIC_URL output and early exit

.github/​workflows/​phala-deploy.yml:260

The new PUBLIC_URL branch is not behaviorally tested: the added workflow test only checks that the source mentions publicUrl, so it would still pass if this branch stopped writing deployment-url or failed to short-circuit the Phala lookup. Add an execution test for phala-url.run with a GITHUB_OUTPUT file and a representative public URL, asserting the normalized output and successful early exit.

@2xburnt
2xburnt marked this pull request as ready for review September 26, 2026 02:55
@2xburnt
2xburnt requested a review from a team September 26, 2026 02:55
@2xburnt
2xburnt merged commit 77f180e into main Sep 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants