UN-4016 [FEAT] Support a platform API key: auth whoami and deployment ls - #3
Open
pk-zipstack wants to merge 4 commits into
Open
UN-4016 [FEAT] Support a platform API key: auth whoami and deployment ls#3pk-zipstack wants to merge 4 commits into
auth whoami and deployment ls#3pk-zipstack wants to merge 4 commits into
Conversation
…ent ls`
Before running anything a user had to assemble a credential out of three values
from three places: `org_id`, a deployment key, and the deployment's `api_name`.
`org_id` had no documented source at all, and there was no way to ask what
deployments exist, so every `api_name` was copied by hand out of the UI.
A platform key carries its own organisation, so supplying the key is enough:
export UNSTRACT_PLATFORM_KEY=...
unstract auth whoami # resolves and stores org_id
unstract docstudio deployment ls # no api_name paste
`platform` is a new product group in the table-driven config layer, which lights
up `config get/set/doctor`, `require()`'s hints and `settings_for()` with no new
code. It deliberately has no `org_id` of its own -- `whoami` writes the resolved
one to the docstudio block, which is where deployment URLs and aliases already
read it from, so there is only ever one copy to keep in agreement.
`PlatformClient` and its `list_api_deployments` already ship in the pinned
client, so `ls` is a thin wrapper. Only `whoami` needed new transport: the
client's `_url` always injects the organisation segment, so a small subclass
builds that one URL itself rather than requiring an upstream release. Its
`PlatformAPIError` is now translated like every other client failure -- without
that a rejected key reached the entry point as a traceback instead of exit 3.
`config doctor --probe` gains a platform branch. Its own docstring lamented
having no side-effect-free endpoint to verify a key against; `whoami` is one. An
absent platform key reports `ok: null` rather than failing, because holding only
a deployment key is the common case and must not decide the exit code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
…the default points Iteration 1 of unstract:remediation against PR #3. Nineteen findings across fifteen classes; one escalated (below). The headline defect had four faces and one cause. `config.py` stated a relationship in a comment -- "the same host as docstudio: one deployment serves both the platform API and the deployments it manages" -- and implemented it as a constant on the very next line. So `platform.base_url` ignored a profile written before the `platform` block existed, ignored `docstudio --base-url`, and left `--api-key` inert for `deployment ls`, sending an organisation-admin platform key to `us-central.unstract.com` after the operator had explicitly named their own host. A reviewer proved it by pointing `--base-url` at 127.0.0.1:9 and getting a real 401 back: a closed port cannot answer, so the request reached the SaaS default. `platform_base_url()` now falls back to the resolved docstudio host, which fixes all four sites at once. The security scan rates it 3/10 as a vulnerability -- vendor host, TLS, and `requests` strips the auth header across hosts -- so it lands as a correctness defect, not a leak. The other four High findings: - `_store_organisation` re-derived the profile ladder and dropped the `$UNSTRACT_PROFILE` tier, so the key resolved from one profile and the org was written into another; the next command then failed after a `whoami` that reported `saved: true`. It now takes `ResolvedConfig.active_profile`, the same chain every read uses. It also refuses to *create* a profile that is not in the file: `setdefault` was materialising a typo, permanently disarming the "Profile not found" guard so every later command silently resolved production defaults. - A failed config write threw away an identity the network call had already returned, exiting 1 ("check your disk") or 2 ("usage error") with `data: null`. `ExitCode.SAVE_FAILED` exists for exactly this and `poll.py` already uses it; the identity now reaches stdout in `details` either way. - `config init` wrote `api_key = "env:UNSTRACT_PLATFORM_KEY"` into every starter profile, and an `env:` reference to an unset variable is a `config doctor` problem -- so doctor exited 1 for every user without a platform key, which this PR's own comment calls the common case. The key is dropped from the starter blocks. - `core/platform.py` -- the only wire-facing new code -- had no test executing it at all, because every command test replaces the factory. Two mutations (breaking the whoami URL, forcing every listing to organisation "") left the suite green. `tests/test_platform.py` now exercises the real class; both mutations fail it. Also fixed: a 204 or non-dict body raised an AttributeError that matched no arm in `__main__`, so the caller got a traceback and no envelope -- the one thing this CLI promises never to do; `requests` transport errors (a scheme-less base_url, a proxy's HTML on a 200) reached the entry point's full-disk handler and were reported as "Check the path and disk."; `PlatformAPIError` folded up to 2KB of server body into `error.message`, which `emit_error` documents as a one-line summary; `--transport-timeout` was accepted on `deployment ls` and ignored (8.65s elapsed against a 1s flag), and `auth` had no such flag at all; `api_path_prefix` was hard-wired, so whoami and ls were unreachable on precisely the self-hosted installs the onprem-example profile caters to, while `clone` worked; `whoami --save` rewrote a discovered project-local `.unstract.toml`, dropping its comments and narrowing its mode; the write was invisible outside `-o json`; the 401 hint talked about deployments on a command that has none; and the README claimed a deployment key "runs one deployment", contradicted by three other statements including this file's own KEY_SOURCES. Every fix is mutation-checked: reverting it fails a named test, nine of nine. ESCALATED, needs a decision: `GET /api/v1/unstract/whoami/` is served only by Zipstack/unstract#2269, which is unmerged. Against any released Unstract the README's documented first command 404s, and `hint_for(404)` sends the reader after a resource id that does not exist. `config doctor --probe` inherits it and exits non-zero on a good setup. Whether this CLI ships before the backend, and what it should say when it does, is a release call rather than a fix. Findings: A B C D E F G H I J K L M N O(escalated) P Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
The first remediation round closed nine findings and opened three of comparable severity, two of them the inverse of the finding they fixed. - `platform_base_url` told "unset" from "chosen" by comparing the resolved value against the built-in default. Those are the same string, so a caller who named the SaaS host was read as having named nothing and redirected to docstudio's -- and `config init` writes that exact host into every profile, so it was the common case. `ResolvedConfig.get_explicit` now answers "did anyone actually name this?" by stopping before the defaults. - `--transport-timeout` was truncated with `int()`, so anything under a second reached urllib3 as 0 and died with a bare ValueError: a traceback and no envelope, which is the failure mode the previous round added a guard to eliminate. The float is passed through; a non-positive value is a usage error about a flag. - Refusing to rewrite a project-local `.unstract.toml` was raised past the SAVE_FAILED wrapper, so it exited 2 and discarded the identity -- on the CLI's documented first command, in any checkout holding the file the README blesses. Declining to save is now a successful call reporting `saved: false`. Also: the unknown-profile guard no longer fires on the "cloud-us" literal it invents when nothing names a profile; a profile typo returns the identity in `details` instead of dropping it; `docstudio --api-key` is refused by `deployment ls` rather than accepted, ignored, and then reported missing; and the duplicated `DEFAULT_PLATFORM_BASE_URL`, a self-contradicting 406 comment, an overstated api_prefix comment, a false `--transport-timeout` help string and a stale README block are corrected. Eight of nine fixes are mutation-pinned: reverting each fails a named test. The ninth -- reading docstudio's tier with `get_explicit` rather than `get` -- is an equivalent mutant while both default hosts are the same string, and is recorded as unpinnable rather than pinned by a test of that coincidence. The `--transport-timeout` wiring the previous round added was covered by nothing; deleting either call site left the suite green. It is covered now. 280 tests pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
pk-zipstack
marked this pull request as ready for review
September 1, 2026 16:42
|
| Filename | Overview |
|---|---|
| src/unstract_cli/core/platform.py | Adds the platform client adapter and tier-aware host resolution; the follow-up resolves the previously reported command-line override failure. |
| src/unstract_cli/commands/platform_cmd.py | Implements organization discovery, safe persistence behavior, and platform-authenticated deployment listing. |
| src/unstract_cli/config.py | Adds platform settings and explicit tier iteration while preserving flag-over-environment-over-profile precedence. |
| src/unstract_cli/core/clients.py | Translates platform and requests-layer failures into consistent CLI error envelopes. |
| src/unstract_cli/commands/config_cmd.py | Extends credential probing to validate an optional platform key through the identity endpoint. |
| tests/test_platform.py | Covers platform URL selection, including the precedence scenario from the previous review thread. |
| tests/test_commands.py | Covers the new command behavior, persistence outcomes, credential errors, timeouts, and deployment listings. |
Sequence Diagram
sequenceDiagram
participant U as User
participant CLI as Unstract CLI
participant P as Platform API
participant C as Config
U->>CLI: auth whoami (platform key)
CLI->>P: GET /api/v1/unstract/whoami/
P-->>CLI: organization identity
CLI->>C: Store docstudio.org_id when allowed
CLI-->>U: Identity and save status
U->>CLI: docstudio deployment ls
CLI->>C: Resolve platform host, key, and org_id
CLI->>P: List API deployments
P-->>CLI: Deployment rows
CLI-->>U: Projected or full listing
Reviews (2): Last reviewed commit: "UN-4016 [FIX] Resolve the platform host ..." | Re-trigger Greptile
Greptile, on PR #3: with `platform.base_url` in the active profile, `docstudio --base-url` was silently ignored and the platform key went to the profile host. `config init` writes `platform.base_url` into every profile it generates, so this was every generated config, not a corner of one. The cause is that `platform_base_url` asked one product for all three of its tiers before asking the other, which inverts the precedence the config layer promises everywhere else: a *profile* value on the preferred product beat a *flag* on the sibling. It walks tier by tier across both products now, so flag > env > profile holds regardless of which product a value was written under; within a tier the platform block still wins as the specific answer. This is a regression the previous commit introduced. Its sentinel comparison happened to mask this path -- a generated `platform.base_url` equals the default, so the sentinel read it as unset and fell through to the flag -- and removing the sentinel to stop discarding a deliberately-named SaaS host exposed it. Both cases now pass for the same reason rather than trading off: the question is which tier named the host, not which product. `ResolvedConfig.explicit_tiers` yields rather than returning a tuple. Built eagerly it read the profile block even when a flag had already answered, which resolves the profile name and raises for one that does not exist -- failing a caller on a tier it never consulted. That is pinned by a test. Eleven resolution paths verified end to end, including the three Greptile named. 283 tests pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
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
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.
What
A
platformproduct group in the config table, and two commands that use it:Based on
feat/cli-scaffold(#2), notmain.Why
Before this, running anything meant assembling a credential from three values in three places:
org_id, a deployment key, and the deployment'sapi_name.org_idhad no documented source at all — it is the first path segment of every web-app URL (useMainAppRoutes.jsx,<Route path=":orgName">) and nothing said so. There was no way to ask what deployments exist, so everyapi_namewas copied by hand out of the UI.A platform key already carries its own organisation, so supplying the key is enough to discover the rest. That is what every comparable pre-OAuth CLI does —
doctl,twilio,sentry-cliall issue one opaque token that carries its scope and is validated the moment it is supplied.How
platformis a fourth entry in the table-driven config layer, which lights upconfig get/set/doctor,require()'s hints andsettings_for()with no new code. It deliberately has noorg_idof its own:auth whoamiwrites the resolved one to thedocstudioblock, which is where deployment URLs and aliases already read it from. Twoorg_idsettings would mean two rows inconfig doctora user has to keep in agreement by hand.deployment lsis a thin wrapper.PlatformClient.list_api_deploymentsalready ships inunstract-client; this adds no transport for it. Rows are projected to five fields by default becauserender_tablewraps rather than truncates and the server sends fifteen —--fullreturns them all, and the help text says so rather than truncating silently.auth whoamineeded one new call.PlatformClient._url()always injects/{prefix}/unstract/{org}/, which is precisely the segment this call exists to discover, soCLIPlatformClientsubclasses it and builds that one URL itself. That avoids an upstream release for a single method; if another consumer wants it, it moves.PlatformAPIErroris now translated like every other client failure. Without that a rejected key reached__main__as an uncaught exception and printed a traceback instead of exiting 3.config doctor --probegains a platform branch. Its own docstring lamented having no side-effect-free endpoint to verify a key against;whoamiis one. An absent platform key reportsok: null, notfalse— holding only a deployment key is the common case and must not decide the command's exit code.Can this PR break any existing features
No new behaviour on any existing path. The config layer gains a product; the three existing ones are untouched, and their env vars, precedence and file layout are unchanged.
clonestill readsUNSTRACT_SRC_PLATFORM_KEY/UNSTRACT_TGT_PLATFORM_KEYand neither theplatformblock nor$UNSTRACT_PLATFORM_KEY— two orgs, so no single profile describes it.Three exact-set assertions in the suite had to grow (
--discover groups, theconfigcommand set, thedeploymentcommand set). Each is a surface addition, not a changed guarantee.Backend dependency
Needs
GET /api/v1/unstract/whoami/, added in Zipstack/unstract#2269. Until that ships,auth whoamiand theconfig doctor --probeplatform row will 404 against a deployment without it.Notes on Testing
280 tests, offline as the rest of the suite is — the client factory is the seam, not the transport. New coverage: the arguments each command hands the client, the envelope and exit code a caller sees, that
whoamiwritesorg_idinto the right profile block and that--no-savewrites nothing, that a rejected key exits 3 rather than raising, and that an absent platform key leavesdoctor --probeat exit 0 while a wrong one fails it.Also exercised live against a local backend carrying #2269, with two seeded organisations: the same commands under two keys return two organisations and two deployment lists, with no
org_idever typed.Review history
Two follow-up commits sit on top of the feature commit, each closing findings from a full pre-merge review pass:
e224089— the platform key was sent to the built-in cloud default rather than the host the caller named, in four places (a legacy profile,docstudio --base-url,--api-key, an explicit[platform] base_url). One resolution helper closed all four.35a41f1— three regressions the first round introduced, two of them the inverse of what they fixed: a sub-second--transport-timeouttruncated to0and died with a traceback rather than an envelope; an explicitly-named SaaS host was discarded because the code inferred "unset" by comparing against the default; and declining to rewrite a project-local.unstract.tomlexited 2 and threw the resolved identity away, on the CLI's documented first command.Every fix is pinned by a test that fails when the fix is reverted, with one exception recorded as an equivalent mutant: reading docstudio's
base_urltier withget_explicitrather thangetis behaviourally identical while both default hosts are the same string, which they are by design.Related
whoamiendpoint this consumes🤖 Generated with Claude Code
https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM