fix(pcl): remove the default platform URL by jacobdcastro · Pull Request #121 · phylaxsystems/pcl

jacobdcastro · 2026-07-31T19:27:26Z

Closes ENG-4201.

pcl defaulted to app.phylax.systems whenever no -u was passed and no platform was remembered. Once app. becomes the network selector, that default points at a host which no longer serves the dApp — and because users upgrade via brew on their own schedule, a bad default outlives the release that removes it.

Rather than repoint the default, this removes it. Any hardcoded default bakes one network into the binary and recreates this bug when the next network ships. A stale interactive list still works; a stale default does not.

Resolution order

-u / --api-url / --auth-url, or PCL_API_URL / PCL_AUTH_URL — accepts any URL, so shadow and staging workflows are unaffected.
The remembered platform.
On a terminal: a one-time pick between Ethereum Mainnet (ethereum.phylax.systems) and Linea Mainnet (linea.phylax.systems), which is remembered.
Nothing resolved, no terminal: a hard error naming -u and PCL_API_URL — never a hanging prompt.

--json counts as non-interactive even from a terminal, so machine output is never corrupted by a prompt.

Why the arg types changed

api_url was a non-Option Url that clap always populated via default_value. That expression is evaluated at command-build time, for every invocation including pcl --help and shell completions — so the picker could not live there without prompting on --help. Removing the default forces Option<Url> across the 7 arg structs, with resolution moved to a single pre-dispatch step in main.rs. The ~31 read sites go through a resolved_api_url() accessor per struct.

Resolving once keeps the prompt one-time, persists a fresh pick through the existing remembered-platform mechanism, and gives every platform-facing command one place to announce its target (Using Linea Mainnet (linea.phylax.systems), stderr, human output only). pcl apply's confirmation names the network too.

Commands that only read local state — auth status, doctor --offline, api manifest, apply --dry-run — resolve a platform when one is already known, but never prompt or fail for one.

Two upgrade consequences for release notes

Every existing production user hits the one-time prompt (or the CI error) on their first post-upgrade run, because production logins were deliberately never recorded.
Credentials with no recorded platform force one re-login. They can no longer be assumed to be production, so the platform-boundary check treats them as a switch. The alternative is sending a token to a platform that did not issue it.

The "production = not remembered" sentinel disappears with the default: remember_platform_url now always records the platform it logged into, and apply's command-line reconstruction pins --api-url only when the user actually passed one.

Worth a close look

auth.rs platform-boundary change — credential_platform returns Option, and an unrecorded platform now counts as switching. This is the security-relevant hunk.
Config persistence in main.rs — a newly chosen platform is written before the command runs. Deferring it to the post-command write loses the choice on any failure and re-prompts on the next run (caught in manual testing; regression test added). The immediate write also refreshes the process-start snapshot, or write_to_file_if_unchanged would silently stop persisting later changes like refreshed auth tokens.
client.rs — the API base URL was already derived from the resolved platform, but nothing locked it in. Extracted to api_base_url with tests asserting dapp./app. never appear: dapp.phylax.systems answers with a 301, and reqwest downgrades a redirected POST to GET, which would silently drop request bodies.
api manifest and --body-template — manifest is exempt from needing a platform (it is static). --body-template runs are not exempt, because the flag lives on ~12 separate workflow arg structs with no shared predicate. Happy to add the accessors if you would rather those worked with no config.

Verification

make ci equivalent all green: fmt, clippy (-D warnings -D clippy::pedantic), full-check, release-check, doc, diff-check, agent-smoke, and 17/17 test suites.

Behaviour also exercised against the built binary, not just tests — first-run picker, sticky second run, non-TTY error, arbitrary -u URLs (shadow + localhost), -u staying one-shot off the login path, and stdout/stderr separation in --json.

Two ACs are out of scope here: end-to-end exercise against the post-cutover hosts, and the release plus minimum-version note on ENG-4178.

🤖 Generated with Claude Code

`pcl` defaulted to app.phylax.systems whenever no `-u` was passed and no platform was remembered. Once app. becomes the network selector that default points at a host which no longer serves the dApp, and because users upgrade via brew on their own schedule a bad default outlives the release that removes it. Rather than repoint the default, remove it. Any hardcoded default bakes one network into the binary and recreates this bug when the next network ships. A stale interactive list still works; a stale default does not. Platform resolution is now explicit, in order: `-u`/`--api-url`/ `--auth-url` or `PCL_API_URL`/`PCL_AUTH_URL`, then the remembered platform, then — on a terminal — a one-time pick between Ethereum Mainnet and Linea Mainnet that is remembered. With nothing resolved and no terminal, commands fail with an error naming `-u` and `PCL_API_URL` instead of hanging on a prompt. `--json` counts as non-interactive even from a terminal, so machine output is never corrupted by a prompt. Resolution happens once before dispatch, which keeps the prompt one-time, persists a fresh pick through the existing remembered-platform mechanism, and gives every platform-facing command a single place to announce its target. Commands that only read local state -- `auth status`, `doctor --offline`, `api manifest`, `apply --dry-run` -- resolve a platform when one is already known but never prompt or fail for one. `-u` still accepts arbitrary URLs, so shadow and staging workflows are unaffected, and off the login path it is a one-shot override that no longer moves the remembered platform. Two consequences worth calling out in release notes: - Every existing production user has no remembered platform, because production logins were deliberately not recorded. All of them get the one-time prompt, or the CI error, on their first run after upgrading. - Credentials with no recorded platform can no longer be assumed to belong to production, so the platform-boundary check treats them as a switch and forces one re-login. Sending a token to a platform that did not issue it is the alternative, which is worse. The "production = not remembered" sentinel disappears with the default: `remember_platform_url` now always records the platform it logged into, and the `pcl apply` command-line reconstruction pins `--api-url` only when the user actually passed one. Also derives the API base URL from the resolved platform through a named helper with regression tests. This was already the behaviour, but nothing locked it in, and dapp.phylax.systems answers with a 301 -- reqwest downgrades a redirected POST to GET, so routing writes through that chain would silently drop request bodies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

linear-code · 2026-07-31T19:27:29Z

ENG-4201

CLAassistant · 2026-07-31T19:27:33Z

All committers have signed the CLA.

phylax-ci

Blocking on the two credential-isolation regressions called out inline: startup overwrites the field used as credential provenance before the platform-boundary check, so both an explicit auth login to B and the legacy credentials-without-platform upgrade path can rebind a token from A and subsequently send it to B. Keep selected/default platform state separate from immutable credential issuer provenance (or invalidate old credentials before persisting the target), and cover both paths with process-level tests that assert no authenticated request occurs.

The remaining inline findings cover concurrent config overwrite/token loss, misleading canonical-network labeling, credential-bearing URL disclosure, invalid-config destruction, JSON error-contract breakage, missed Linea V1 compatibility warnings, and local dry-run/template commands that now unnecessarily require or persist a platform.

Validation: 16 platform unit tests plus focused auth/assertion-spec tests pass on a5bde1b. GitHub test/lint, Socket security, release, docs, and smoke checks pass; cargo-deny separately reports RUSTSEC-2026-0220 for ruint 1.18.0, which is not introduced by this PR because Cargo.lock is unchanged.

jacobdcastro · 2026-08-03T18:32:35Z

@makemake-kbo — one question to close out a review finding here.

Which assertion spec runs behind each new platform host?

linea.phylax.systems — V1 only, or V2-capable?
ethereum.phylax.systems — V1 only, or V2-capable?

Why it matters: assertion_spec.rs still has V1_ONLY_PLATFORM_HOSTS = ["app.phylax.systems"], so pcl auth login against either new host prints no V1/V2 compatibility notice. And now that app. is only a router rather than a platform deployment, that list points at a host that no longer serves the API. The chain-id check (Linea 59144/59141) still fires on pcl deploy, so the gap is login plus any path where the chain isn't known yet.

What I'll do with each answer:

linea V1, ethereum V2 → host list becomes ["linea.phylax.systems", "app.phylax.systems"]
both V1 → add both new hosts
both V2 → drop the host check entirely; only the chain-id check stays

Leaving this finding open and fixing the other 10 in the meantime.

odyslam · 2026-08-03T18:35:32Z

@jacobdcastro Ethereum is V2, Linea is V1 for now.

We should make a note that we need a better way to manage this and not allow users to deploy assertions to a network that don't support their exact versions.

Separates credential provenance from platform selection, keeps the startup config write inside the error boundary, stops mislabelling non-canonical URLs, and restores platform independence for local commands. Credential isolation. `credential_platform()` read `config.platform_url`, which is selection state: startup overwrites it from an explicit `-u` before the command runs, and an interactive pick writes it before dispatch. Either could rebind a stored token to a platform that never issued it, and the boundary check would then compare the target against itself. Two concrete leaks followed — `pcl auth --auth-url B login` with a valid token from A short-circuited as "already authenticated" and left A's token bound to B, and the documented legacy upgrade path silently skipped its one-time re-login as soon as a network was selected. Provenance now lives on the credentials themselves as `UserAuth::issuer_platform_url`, written only when fresh credentials are stored and carried through a refresh, so the invariant is structural rather than dependent on startup ordering. Startup config write. The pre-command write escaped the structured error boundary, so a `--json` run that could not write its config emitted a color-eyre diagnostic instead of a single error envelope. It also replaced a config it had failed to parse before the repair command had produced anything, letting a cancelled or failed login destroy recoverable credentials and RPC settings; the original bytes are now preserved at `config.toml.invalid`. Finally, it wrote a snapshot taken before the selector prompted, which could roll back a refresh-token rotation performed by another process while the prompt sat open — the platform is merged into the current file instead. Platform display. `Network::from_url` matched on host alone, so `http://linea.phylax.systems:8080` and `https://user:secret@linea.phylax.systems` were both announced as "Linea Mainnet" despite reaching a different target; matching is now on the full canonical origin. Platform URLs rendered for humans go through a redacting formatter, since the per-command announcement copied any embedded password into terminal scrollback and CI logs. Userinfo and query are masked rather than dropped, so two different targets never render alike. Storage and the boundary comparison stay lossless. Local commands. `export incidents --dry-run`, `deploy --dry-run`, and every `--body-template` workflow return before building a client, but all were classified as needing a platform and so failed with `platform.not_selected` on a clean non-interactive install — breaking the agent-discovery path these commands exist to serve. Each now carries a `needs_platform_url` predicate, and the incident-export resume command omits `--api-url` when no platform was ever chosen. The workflow-template tests are back to running against an empty config instead of injecting `PCL_API_URL`, which is what hid the regression. Assertion spec. `V1_ONLY_PLATFORM_HOSTS` still named only `app.phylax.systems`, which is now a router page rather than a platform, so a login to `linea.phylax.systems` got no V1/V2 compatibility notice. Linea runs V1 and Ethereum runs V2; a test now pins the capability of every network the selector offers so a rename cannot silently drop the notice again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`cargo deny check advisories` was failing on a DoS-class advisory in ruint 1.18.0: `overflowing_shl`/`overflowing_shr` returned false-negative overflow flags, which propagates into `checked_*` returning `Some` instead of `None` and can make `to_base_be` (and string formatting) loop forever on no-alloc builds for non-limb-aligned widths. 1.20.0 is the patched release and semver-compatible, so this is a lockfile bump with no manifest change. Preferred over a `deny.toml` ignore because a fixed version exists — the existing ignores are all cases with no safe upgrade, and deny.toml asks that new ones record an upstream owner. ruint gains an optional `ark-ff-06` dependency, so the lockfile picks up ark-ff 0.6.0 and its arkworks companions alongside the 0.3/0.4/0.5 entries already there. None of them are compiled: this workspace enables ruint's alloy-rlp, arbitrary, num-integer, num-traits, proptest, rand-09, and serde features, and no ark-ff feature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

phylax-ci

The two P1 credential-isolation bugs are fixed: issuer provenance now lives with the stored token, refresh preserves it, and authenticated client construction enforces it. The canonical-network labeling, URL redaction, invalid-config backup, structured JSON failure, Linea V1 warning, deploy dry-run, incident-export dry-run, and most local template fixes also look correct. I am still requesting changes for one overlapping config-write race and three missed local template paths called out inline. GitHub test/lint, cargo-deny, Socket security, release, docs, and agent-smoke checks are green on 82c1335; the CLA check remains pending.

Three review findings on the platform-selection work. Recording a chosen platform reloaded the config before merging, which only protects writes that finished before the reload. A token refresh landing between that read and the write was silently overwritten, discarding the only valid refresh token. Refresh already held a lock across its own read-modify-write, so promote that lock to a shared `ConfigLock` in the config module and take it for the platform merge too. The two operations are now mutually exclusive rather than merely narrow. `--body-template` prints a compiled-in schema and must work on a clean install with no platform selected. Three commands still demanded one: `projects update`, `access role update`, and `access member remove`. All three carry the flag on a flattened or nested args struct, so the outer subcommand variant has no `body_template` field of its own and fell through the `_ => true` catch-all. Match every variant exhaustively instead, in projects, releases, and access alike, so a new body-taking variant fails to compile rather than quietly requiring a platform. Regression coverage for the overlap uses the lock as a barrier: the merge is forced to wait while a rotation is written, which reproduces the interleaving a pre-staged rotation cannot. Verified it fails with the lock removed. The template test now covers all 19 invocations that accept `--body-template`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

phylax-ci

Approved on 123075e. The shared config lock closes the refresh-versus-platform merge race, and the exhaustive project/access predicates plus the full local-template invocation matrix cover the three missed paths. Local validation passed: both selected-platform merge tests, the lock-timeout test, and the 20-command no-config template process test.

jacobdcastro requested review from lean-apple and makemake-kbo July 31, 2026 19:27