Skip to content

TC-1482: wire optional NODE_STATS_READ_TOKEN into the production deploy (companion to #370) - #371

Merged
samgbafa merged 9 commits into
mainfrom
feat/tc-1482-deploy-wiring
Oct 19, 2026
Merged

samgbafa merged 9 commits into
mainfrom
feat/tc-1482-deploy-wiring

Conversation

@samgbafa

@samgbafa samgbafa commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

TC-1482 deploy wiring, companion to #370 (GET /admin/stats, bearer NODE_STATS_READ_TOKEN).

Base: main. Ships with #370 in the release after 1.21.2; review together. Not for 1.21.2. Do not merge before #370. No deploy, Phala call or Docker was run for this PR.

Compose hash changes

docker-compose.dstack-postgres.yaml gains one line, so the reviewed N0 template lock changes:

raw compose template (node digest = 64 zeros)
main 1228d81927fc0a3fbba85745103e77b3112606f5d314d15974d400b222409350 7fabba70e71423faa2c8b61981d4a7c95ac7c2552074af81c0877af774d6b383
this PR 8f67b3b25136537c65b154b8729b0a1a7d9911b9963acc121a303cb9d806a6e7 ae497fed720da12219999f4bb8573fb81b235c25086c96142e87f11e948b5832

docker.yml n0_template_hash and scripts/ci/fixtures/tc959-n0-compose.yaml (byte-identical to the compose) move with it. ops/rollback/* hashes are unchanged. Release notes for that release must say the compose hash changed.

Files and where the line was added

  • docker-compose.dstack-postgres.yaml: tinycloud.environment, directly after TINYCLOUD_ADMIN_SECRET: NODE_STATS_READ_TOKEN: "${NODE_STATS_READ_TOKEN:-}". The :- form (already used in this file) makes it optional and warning-free when unset. This is the only compose edit; it is the only compose file that passes the admin secret.
  • scripts/ci/fixtures/tc959-n0-compose.yaml: same line (test fixture copy of the compose).
  • .github/workflows/docker.yml, job deploy-phala:
    • n0_template_hash and its comment (validate step).
    • Deploy to Phala Cloud env: PHALA_ENV_NODE_STATS_READ_TOKEN: ${{ secrets.PROD_TINYCLOUD_NODE_STATS_READ_TOKEN }}, after PHALA_ENV_TINYCLOUD_ADMIN_SECRET.
    • Not added to required_vars. A separate single-line check (CR/LF fails before any CVM mutation) after that loop.
    • .phala-prod.env: NODE_STATS_READ_TOKEN= is written only when the value is non-empty.
  • docs/releasing.md: new section NODE_STATS_READ_TOKEN (TC-1482): compose hash change (after the quota/billing paragraph, ~l.150); template/raw hash in the N0 requirements list and the 12c recovery paragraph.
  • docs/runbooks/tc-959-n0-log-shipping.md: hash table row.
  • .github/workflows/docker.yml (review round 1, L2): when the token is set, the step refuses, before writing .phala-prod.env, a token equal to PHALA_ENV_TINYCLOUD_ADMIN_SECRET (::error::NODE_STATS_READ_TOKEN must differ from TINYCLOUD_ADMIN_SECRET) and a token shorter than 32 bytes (::error::NODE_STATS_READ_TOKEN must be at least 32 bytes; surrounding whitespace not counted; wc -c bytes). Constant messages, no value printed. Consistent with feat(admin): read-only GET /admin/stats owner list (TC-1482) #370, where such a token makes the node keep the route at 404.
  • scripts/ci/test-deploy-recovery.py: unset/empty token does not block the deploy and is absent from the sealed file; set token is sealed once and never appears in logs; a multi-line token fails before phala deploy. New (L2): equal-to-admin, 31-byte, whitespace-padded 31-byte, whitespace-only and 30-byte multibyte tokens are refused before any CVM mutation and before the sealed file is written; 32-byte, 64-hex and 32-byte multibyte tokens pass (mutation check: removing both guards fails all five refusal cases). New (L3): stub mode where the CVM's allowed_envs already names NODE_STATS_READ_TOKEN and the secret is unset or empty: success False, no cvm-deploy, message ::error::allowed_envs synchronization failed during complete whitelist validation; response details withheld.; the same for the pre-N0 rollback path, which passes with the secret present. The stub stands in for sync.mjs --check (the test also asserts the real message and phase string exist in docker.yml).

Secret handling

Same path as TINYCLOUD_ADMIN_SECRET: GitHub production environment secret, then the step env:, then the 0600 .phala-prod.env (removed afterwards), then phala deploy -e <file>. The value is never in a repo file, argv or logs (argv carries only the file name). That existing path already keeps secrets off argv; nothing was copied from a pattern that does not.

Unset token

Optional: the line is not written, docker compose config --quiet still passes (:-), --validate-config does not read it, and the node starts with /admin/stats disabled (404, per #370). Once a deploy has put the name in the CVM allowed_envs, later deploys must keep supplying it (existing whitelist check).

Rollback files

ops/rollback/pre-n0-compose.yaml and pre-n0-stop-local-logs.yaml are unchanged: they are pinned by raw hash, restore the 1.20.6 image which does not read the variable, and rollback therefore leaves the route absent.

Operator steps (do not do before review)

Bobbie generates a 32-byte hex token (openssl rand -hex 32, 64 characters); Amos stores it as the PROD_TINYCLOUD_NODE_STATS_READ_TOKEN environment secret (stdin, never printed) so the release deploy seals it into the Phala env; the same value is set as OpenKey's NODE_STATS_READ_TOKEN. It must differ from PROD_TINYCLOUD_ADMIN_SECRET and be at least 32 bytes; the deploy refuses otherwise. docs/releasing.md carries the release-note items (registration/KMS, node DID, never-delete rule, double restart of the first deploy, token generation).

Checks run

python3 scripts/ci/test-deploy-recovery.py (25 tests, offline stubs) OK; actionlint on docker.yml reports the same two shellcheck findings as main; YAML parse of all compose files; node --test for the logship suites (33) OK. docker compose config was not run (no Docker).

Overlap

The #370 bullet in docs/releasing.md (~l.112-119, "not passed to the CVM by docker.yml...") lives only on #370's branch, so #370 owns it; it is stale once both land, and whoever merges second updates it. #370 edits docs/releasing.md at ~l.109-119 and README/CHANGELOG; this PR does not touch those hunks (a trial merge with #370 had no conflicts; the #370 bullet saying the token is not wired will be stale once both land). Older open PRs #342, #288, #325, #272, #231 also touch compose/docker.yml but predate main's current compose.

dryrun added 2 commits October 19, 2026 13:11
Companion to #370. Optional sealed env value, compose environment line,
N0 template hash and operator docs. Unset keeps GET /admin/stats disabled.
Adds the /admin/stats route label to the log-shipper field list (copy of the
tinycloud-dev canonical list), the ingress route map and the Vector boundary
cases. Re-pins log-shipper/canonical.json to the tinycloud-dev PR head, so the
pin MUST be updated after that PR merges. Regenerates the embedded Vector
config in the N0 compose and its fixture, and moves the N0 template lock.
Route template plus status only; no query, credential, body or address (TC-961).
# Conflicts:
#	.github/workflows/docker.yml
#	docs/releasing.md
#	docs/runbooks/tc-959-n0-log-shipping.md
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1482 (reviewed exact head fe16c31)

Independent security review (Opus gate) of exact head fe16c31928650933d08e991322ecf5a095b073e3. Condition: do not merge before #370's H1 is fixed (#370 is CHANGES NEEDED: it counts every bootstrapped OpenKey account).

Hashes, recomputed with docker.yml's own n0_template method:

  • compose (base 9573f0e): raw 1228d819… → 8f67b3b25136537c65b154b8729b0a1a7d9911b9963acc121a303cb9d806a6e7, template 7fabba70… → ae497fed720da12219999f4bb8573fb81b235c25086c96142e87f11e948b5832.
  • The fixture is byte-identical. The compose diff is exactly one line.
  • docker.yml:672, releasing.md:344-345/414 and runbook:150 all agree. No live pin is left on the old values; CHANGELOG keeps them as history.

Secrets: environment secret → step env → printf into the 0600 .phala-prod.env (stdout redirected) → phala deploy -e with output silenced → trap/always rm. No argv, no echo, set +x, constant error text, no artifact upload. A multi-line value fails before any CVM mutation; removing the new check makes the +1 test fail (mutation-checked).

Release-note answers (Naomi):

  • (a) Registration before deploy: none in our process, if production still uses Phala's centralized KMS (docker.yml:504-508). Amos must confirm with phala cvms get before deploying.
    • With an on-chain KMS, a reviewed addComposeHash is required first. Otherwise the KMS answers "Boot denied", there are no keys and the node does not start.
    • The KMS and attestation key on the dstack app-compose compose_hash (currently 4bc25b85…). That is SHA-256 of app-compose.json, which includes allowed_envs, so it changes at deploy and again when the first deploy's env sync adds the name.
    • Not on the raw/template hashes (repo-internal), and not on ca110b0b… (the raw hash of Add a compose-only node log sidecar #309's compose; nothing gates on it).
    • TC-887: not a blocker (ceremony not held, at-rest off). Record the new compose_hash/RTMR3 after the deploy, and rebuild any draft trust.json.
  • (b) DID: unchanged. The node sends /GetKey {"path":"tinycloud/keys/primary"} only (dstack.rs:138-146, lib.rs:1278). The key is derive(app_key, [path]) (guest-agent rpc_service.rs:645-668; purpose is only in the signature), and app_key = KMS derive([app_id, "app-key"]) (kms crypto.rs:11-16). The compose hash is not an input; only the quote/RTMR3/tee_binding_digest change.
    • Confidence: high. Not verifiable without a live CVM: that Phala's hosted KMS derives exactly as upstream.
    • Precedents: DID unchanged across the 1.21.0 compose change and the 0.5.12 PATCH. The post-deploy DID + GetKey canary gate stays.
  • (c) A later deploy omits the token: fails closed before any CVM mutation. sync.mjs --check (docker.yml:1013) evaluates requireCondition(allowed.every((key) => keys.has(key))) (l.862) and prints ::error::allowed_envs synchronization failed during complete whitelist validation; response details withheld. (l.913).
    • The pre-N0 rollback and the recovery use the same step, so a deleted secret also blocks rollback.
    • The workflow can only add names (l.876). Removing one is a manual, reviewed control-plane allowed_envs update first, then deleting the secret.
    • The first deploy with the token restarts the CVM twice: dstack skips the not-yet-allowed key, then the env-sync PATCH restarts.

Low: docs should carry the (c) message, the rollback impact, the removal steps, the double restart and hex tokens. No workflow refusal of stats token == admin secret. The +1 test stubs node, so the "allowed name missing" path is not exercised.

CI at fe16c31 when posting: pass: Build dstack ingress image, Version guard, at-rest harness, boundary, build (workspace), capabilities, clippy (workspace), fmt (workspace), hosted-compose-proof, m1-realdata, plan, separate workspace lockfiles, w5 workspace lockfile. Pending: build, test (workspace, PostgreSQL-backed). Skipped: announce, build-dstack, build-global-artifacts, build-local-artifacts, host, publish-homebrew-formula, rollback-audit. The verdict is code-only for the pending checks.

…ment the release notes (TC-1482)

- docker.yml: when PROD_TINYCLOUD_NODE_STATS_READ_TOKEN is set, refuse a value
  equal to the admin secret or shorter than 32 bytes before the sealed file is
  written, with constant messages that name no value.
- test-deploy-recovery.py: cover both refusals, and the stub mode where the
  CVM's allowed_envs already names NODE_STATS_READ_TOKEN and the secret is
  unset (fails before any CVM mutation, also for the rollback path).
- docs/releasing.md: release-note items for the compose-hash change, the node
  DID, the never-delete rule, the double restart of the first deploy, and the
  token generation.
@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1482 (reviewed exact head 03810e2)

@samgbafa

Copy link
Copy Markdown
Contributor Author

APPROVE: see TC-1482 (main-merge only; +/- lines byte-identical vs ed81bd1 (= the merged #372 tree, 9321dbf); head 68ca445)

Detail: merged origin/main a742ab8 (1.21.2) into 9321dbf with a merge commit (d395a84). The only conflict was CHANGELOG.md: this branch's Unreleased entry and main's [1.21.2] section sat at the same spot; resolved by keeping both (Unreleased first, then [1.21.2] untouched). The PR diff against main has the same 13 files as before; the patch-id differs (78df835e… vs 114b21eb…) only because main's 1.21.2 section moved the CHANGELOG context; the added/removed lines are byte-identical per file for all 13 files (sha256 of the +/- lines compared per file: docker.yml, CHANGELOG.md, redacted-logging.conf, test-redacted-logging.sh, the compose, docs/releasing.md, the tc-959 runbook, canonical.json, cases.mjs, diagnostics.json, fields.json, the fixture, test-deploy-recovery.py). The compose and its hashes are unchanged (raw 32a002ef…, template 79469f97…).
Second commit 68ca445 (waiver for the #372 re-pin): log-shipper/canonical.json revision ee823cd5 -> 9bda95fa (tinycloud-dev #163 merge commit; ee823cd5 is its ancestor); sha256 unchanged (cff33bf5…ce6d8, verified equal to ops/logging/fields.json at 9bda95fa and to log-shipper/fields.json); that one line is the only change in the commit; no other mention of the old revision exists in the repo.
Local: test-deploy-recovery.py 25 OK; log-shipper budget/profile node tests 11 pass. CI must be green at this exact head before merge (the full ci / Docker Container / logship / hosted-compose-proof run on this main-based head for the first time).

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.

1 participant