UN-4016 [FEAT] Resolve the organisation from a platform API key via a whoami endpoint - #2269
Merged
Merged
Conversation
praveen-formido
added a commit
that referenced
this pull request
Sep 1, 2026
…endpoint sends Iteration 1 of unstract:remediation against PR #2269. Fifteen of sixteen findings; F6 is handed back (see below). The two High findings were both invisible from inside the change: F1 — the spec published `ErrorResponse` for 401/403, but `whoami` is deliberately not whitelisted, so `CustomAuthMiddleware` answers every rejection itself with a bare `{"message": ...}` and DRF's exception handler is never reached. A generated client branching on `errors[0].code` would raise on the most common failure the endpoint has. Now declares a `PlatformKeyError` shape that matches the wire, with the 500 carrying no body schema because a Django HTML 500 has none. F2 — `@pytest.mark.critical_path("platform-key-whoami")` named an id in no registry, and `tests/rig/cli.py` sets `overall_exit = 1` on an unknown marker. The rig would have failed the build on every run. CI never caught it because `test` skips on draft PRs. Now registered. Also fixed: the view returned 403 where the spec said 401 (DRF coerces `NotAuthenticated` unless the first authenticator offers a WWW-Authenticate header, and SessionAuthentication offers none) — it now returns 401 explicitly, and the branch has a test, which it never had because every other rejection is answered before the view runs. The whitelist regex is anchored, so an organisation named `whoami` no longer has its whole API treated as organisation-less. The organisation-scoped alias `/<org>/whoami/`, which the mount comment wrongly claimed could not exist, is now documented and tested in both directions. The dominant defect class was not any single bug: seven of sixteen findings were confidently-worded comments asserting mechanisms the code does not implement — that DRF resolves no user, that the org FK is non-null by construction, that no rewrite can produce `whoami/`, that a test observes middleware it never invokes. Each is corrected to what the code does, or deleted. Every fix is mutation-checked: reverting it fails a named test. That check also caught one of the new tests passing vacuously — it matched an error string the OpenAPI-validity gate produces, so it went green with the gate it was written for removed. F6 is not fixed here and needs a decision: setting `organization_id = None` makes `whoami` bypass `SubscriptionMiddleware` in the enterprise tree, where every other org-less path has an explicit `SUBSCRIPTION_WHITELISTED_PATHS_LIST` entry. The fix belongs in unstract-cloud and the intent is a product call. Findings: F1 F2 F3 F4 F5 F7(partial) F8 F9 F10 F11 F12 F13 F14 F15 F16 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
Contributor
Author
|
@chandrasekharan-zipstack @hari-kuriakose — one finding from this PR's review needs a decision from you, because the fix isn't in this repo.
|
praveen-formido
added a commit
to Zipstack/unstract-cli
that referenced
this pull request
Sep 1, 2026
…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
praveen-formido
marked this pull request as ready for review
September 2, 2026 03:11
Contributor
 |
praveen-formido
requested review from
chandrasekharan-zipstack and
hari-kuriakose
and removed request for
chandrasekharan-zipstack
September 2, 2026 03:23
hari-kuriakose
requested changes
Sep 2, 2026
hari-kuriakose
left a comment
hari-kuriakose
left a comment
Contributor
There was a problem hiding this comment.
@pk-zipstack Why do we even need to enumerate org from the API key in the client? When you pass a platform API key, the corresponding org id should be retrieved in the backend automatically.
Contributor
Author
|
@hari-kuriakose You're right, and the code backs you more strongly than I'd assumed when I wrote this. I went and checked before answering. The org id in the URL is not a source of truth anywhere in this middleware — it is only an assertion. The org that actually scopes a request comes from the credential in both auth paths:
And everything downstream reads that, not the URL: The only use of the URL org is the mismatch check at What I think that means The cleaner design is the one you're pointing at: let a platform-key-authenticated request omit the org segment and resolve it from the key — which is exactly what this PR's Three things worth deciding rather than assuming, which is why I'm not just doing it:
So, the question back to you and @chandrasekharan-zipstack @Deepak-Kesavan: do you want org-less aliases for the platform-key endpoints as a follow-up — and if so, all of them or just the deployment ones? I'm happy to do that work; I'd rather agree the API surface with you first than guess at it. In the meantime this PR is self-contained and its value doesn't depend on the answer. Happy to hold it if you'd rather land the whole shape at once. Co-authored with Claude Code. Every file and line reference above was verified against |
Contributor
@hari-kuriakose this is mainly to ease friction of configuring creds for the CLI. Currently, users would have to paste org_id, API keys (both Unstract and LLMW separate) and deployment endpoints to a config file. With this Long-term, supporting OAuth might be an even better experience |
Contributor
@chandrasekharan-zipstack Sorry, didn't get you still. With just the Platform API key given as input, shouldn't we be able to get a list of all API Deployments then auto provision a Global API Deployment key and proceed from there? Why do we need this new endpoint? OAuth login is required eventually but that is a separate and secure API anyways. |
Contributor
@hari-kuriakose in order to list the API deployments -> we need both platform API key + org_id (this is because org_id is needed to frame the URL to list deployments).
|
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
Contributor
There was a problem hiding this comment.
Review — 4 High, 8 Medium
Reviewed against head 245672ae9. Findings are inline. 13 Low findings (mostly comment/doc accuracy and uncovered branches) are omitted here and can be shared separately if useful.
The core design is sound: the org is derived from key.organization, tenant isolation holds on both URL forms, and the alias correctly re-arms the belongs-to-org check. The findings below are about the edges — cloud behaviour, the accuracy of the comments after the last commit, and two tests that do not currently constrain anything.
Checked and clear, so nobody re-derives them:
- The
$anchor on the whitelist regex is correct and does not break the alias — the whitelist runs on the un-rewrittenrequest.path, so the alias correctly fails it and falls through to the rewrite. /api/v1/unstract/whoami(no trailing slash) does not produce a misleading 403. It fails the org pattern entirely (which requires a/after the org segment), soorganization_idstaysNoneandCommonMiddlewareAPPEND_SLASH redirects.- The spec-test narrowing from "every operation" to "deployment operations" is net stronger, not weaker — the added else-branches constrain
whoamimore tightly than the original did. - No concurrency concerns:
StateStoreisthreading.local()and cleared in afinally. - No added queries:
select_related("organization")already covers the view's single read.
hari-kuriakose
left a comment
hari-kuriakose
left a comment
Contributor
There was a problem hiding this comment.
Review — REQUEST-CHANGES · 0 Critical, 1 High, 6 Medium, 3 Low
Reviewed at head 245672ae9 against merge-base c49968e3e (13 files, 895 lines), under the team's standardized 19-lens rubric. Findings are inline.
This is a follow-up to my review of 2026-09-02. Reconciliation of that finding is below.
The core design is sound, and I verified it rather than assuming it: tenant isolation holds. The view reads key.organization, and custom_auth_middleware.py:140 stamps the StateStore org from the key row, never from the URL. The endpoint genuinely authenticates — it is deliberately absent from WHITELISTED_PATHS, and a test pins that. The alias re-arms the key-belongs-to-org check and is tested on the wire, body included. The test suite is well above average for this repo: it asserts response bodies rather than statuses alone, and its docstrings are candid about what they do not cover.
The change request is driven by the spec-gate regression and the accompanying claim in the PR description, not by the finding count — most of which is comment accuracy.
My prior finding — STILL-OPEN
I asked: "Why do we even need to enumerate org from the API key in the client? When you pass a platform API key, the corresponding org id should be retrieved in the backend automatically."
Reading the code now: the premise was correct. custom_auth_middleware.py:140 already resolves the organisation from the key on every Bearer request, with no client involvement.
What this PR adds is orthogonal to that. Org-scoped routes are mounted under /<org>/, and OrganizationMiddleware parses the organisation out of the path, so a client still needs the string in order to construct those URLs — even though the backend independently knows it. The endpoint therefore solves a real and distinct problem (URL construction).
It does not, however, answer the question I asked: whether org-scoped routes should accept a platform key without the org segment, given the backend already resolves it. That is a larger design decision which this PR neither forecloses nor addresses. Not blocking — recorded so it is not lost.
Unanchored findings
1. The PR description states a property the code no longer has. It says the published-prefix check is "still rejecting an overridden API_DEPLOYMENT_PATH_PREFIX". For any override under the tenant mount it now accepts it — see the inline finding on generate_docstudio_spec.py:33. Worth correcting in the description as well as the code, since this is the artifact reviewers read first.
2. The unstract-cloud subscription-gate question — @praveen-formido asked me by name, so here is an answer rather than a deferral. Exemption is defensible: an identity probe that tells you which org your key belongs to should arguably answer regardless of billing state, and it reads nothing billable. But it should be an explicit SUBSCRIPTION_WHITELISTED_PATHS_LIST entry with a comment like its neighbours (marketplace/webhook/, tackle/webhook/, …), not a side effect of organization_id being None — that breaks the day get_subscription stops tolerating a null org, and it silently issues a Subscription.objects.get(organization_id=None) query on every call. I cannot verify current cloud state from this repo, so this is a recommendation on the decision, not a verification of the code.
3. Lens 13 coverage is split. The whoami suite could not be executed in review — APITestCase + conftest.py:26-44 auto-marks it integration, requiring live Postgres. 17 tests collected, 0 run. The reasoning about the suite is sound and produced a real finding, but no wire behaviour was exercised by this review; CI remains the only evidence there.
Lens checklist (19/19)
| # | Lens | Status |
|---|---|---|
| 1 | Correctness | Clean |
| 2 | Design & fit | See findings |
| 3 | Error handling | Clean — explicit clean report from the specialist |
| 4 | Security | Not covered — deep-dive ran DEGRADED (finder sub-task never returned; ran inline, filter stage vacuous). Surface read directly by me |
| 5 | Data integrity | N/A — no migrations, no model changes |
| 6 | Concurrency | N/A — no primitives added |
| 7 | API compatibility | See findings |
| 8 | Reliability | Clean — no timeouts/retries/external calls added; single in-memory attribute read on an already-select_related row |
| 9 | Performance & cost | Clean — no added queries |
| 10 | Observability | Clean |
| 11 | Operational safety | Clean |
| 12 | LLM/agent | N/A |
| 13 | Tests | Not covered — Postgres unreachable, 17 collected / 0 run (findings still filed) |
| 14 | Dependencies & build | N/A — no dependency or lockfile changes |
| 15 | Guideline compliance | Clean — pinned ruff 0.3.4 check and format --check both pass |
| 16 | Doc accuracy | See findings |
| 17 | Cross-document consistency | Clean — merge-result sweep ran (branch is 8 commits behind main); no merge-created divergence |
| 18 | On-prem experience | N/A — no charts; the setting is absent from every env sample and values schema |
| 19 | Scaling & workload lifecycle | N/A |
boundary-sweep: 5 sinks traced, clean — four response fields plus the 401 literal. The diff adds no log lines at all. Every field is read off the caller's own key row, so the viewer is entitled to it by construction; no key material reaches the response.
Checked and clear — so nobody re-derives them
- The published spec does not contradict itself. I initially suspected
whoami's 401/403 declaredPlatformKeyErrorwhile carrying{type, errors[]}examples. That was my own analysis error — a script that misattributed deployment-operation examples towhoami. Corrected:whoami401/403 carry no examples.PlatformKeyAutoSchemaworks as designed. test_no_published_example_contradicts_its_own_schemais not vacuous. It executes zero assertions today, but that is what a for-all check looks like when its invariant holds. Settled by mutation rather than reading: deletePlatformKeyAutoSchema._get_examples, regenerate, and the whoami 401/403 gain examples, the loop body executes, and the test fails. It binds exactly the property it advertises.- The spec is not stale —
openapi_schema.pyandspecs/docstudio-oss.jsonwere regenerated together in245672ae9. - The spec change is purely additive (112 insertions, 0 deletions), so the deployment operations are byte-identical and no downstream client PR is forced by this change.
- Nothing is shadowed by the new mount — no tenant urlconf declares
whoami/. - The
$anchor is correct, and/api/v1/unstract/whoami(no trailing slash) does not produce a misleading 403. - The
ruffI001 you may see when lintingtest_whoami.pyby explicit path is an exclusion artifact, not a real violation —--force-excludereports "No Python files found", so pre-commit skips it.
Standardized 19-lens review · unstract:standard-review · 7 agents · scoped to git diff c49968e3e04c0cfd0254fd936d09aee14c0eefd8 245672ae9
Contributor
|
Update — lens 4 (Security) upgraded from When I posted the review above, the security deep-dive's finder sub-task had failed to return, so the pass ran degraded and I reported lens 4 honestly as Result: no findings at confidence >= 8. Nine candidates were enumerated and each fell below the floor with evidence — the newly-live whitelist branch (exactly one runtime consumer, no settings module redefines it), the The substantive point on the guard short-circuit is worth restating, because it is stronger than "the skip is compensated": nothing is skipped that had anything to check. This remains a precision-biased scan, not a clean bill of health: its hard exclusions mean DoS, rate limiting, resource exhaustion and dependency currency were never candidates. The unthrottled-key-oracle observation in my Low findings above sits in exactly that excluded space, so it stands on its own footing rather than being cleared by this scan. One small correction to a comment, surfaced by the re-run.
re.match(r"^/api/v1/unstract/whoami/$", "/api/v1/unstract/whoami/\n") # matches
re.match(r"^/api/v1/unstract/whoami/$", "/api/v1/unstract/whoami/\nfoo") # does notNo security consequence, which is why it is not in the findings list: the newline variant resolves to the same view with the same key-derived organisation, and nothing can be smuggled after the newline. The anchor's actual purpose — stopping an organisation literally named If you want the comment to be exactly true, Follow-up to the standardized 19-lens review above. Lens 4 now reads: Clean — completed find pass, no findings at confidence >= 8, within the scan's stated exclusions. |
|
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
praveen-formido
pushed a commit
that referenced
this pull request
Sep 9, 2026
…bout them Greptile, on PR #2269. The rejection logging added in 0e9a23a took the first `X-Forwarded-For` value as the caller address. Nothing in this project validates a forwarding chain, so that value is supplied by the party being logged -- which defeats the only purpose those lines have, and puts an unvalidated header into the log stream. `REMOTE_ADDR` only now, matching `internal_api_auth.py` and `internal_base_urls.py`, the two other places that log a caller address. Behind a proxy this records the proxy; recovering the real client needs a trusted-proxy setting this project does not have, and guessing a hop count here would be the same mistake in a different shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
Contributor
Author
Review response —
|
| Finding | Resolution |
|---|---|
Spec gate accepted an overridden API_DEPLOYMENT_PATH_PREFIX |
PUBLISHED_PATH_PREFIXES now lists each route, not the mount. Verified: @hari-kuriakose's exact case now fails the gate; unset, the spec regenerates byte-identically |
openapi_schema.py docstring claimed two published error shapes |
Deleted; replaced with the published shape plus an explicit wire/spec separation |
| 403 unreachable on the published path | Withdrawn. Moved to the deployment assertion beside 400/404; universal set is now {401, 500}, which resolves the contradiction between the suite's own two rules |
| Middleware pin hid the ordering the route depends on | Ordering now asserted against the shipped setting, outside the override. Mutation-checked: swap the two entries and all 17 existing tests stay green while only the new assertion fails |
"cloud test settings drop CustomAuthMiddleware" |
False, and corrected. unstract-cloud/…/test_cloud.py:7 says "No middleware is excluded." Chandru was right |
| Whitelist re-spelled the mount, unescaped env value | rf"^/{re.escape(TENANT_SUBFOLDER_PREFIX)}/whoami/$" |
| Example-divergence allow-list coarse and un-self-checking | Keyed by schema name; asserts its entries still match. It immediately caught three wrong schema names in my own first draft |
| That check ran zero assertions | Positive case added |
MCP organization holds an id, not a name |
Documented, with the direction of the trap |
Missing WWW-Authenticate on 401 |
Added, spelled as mcp_server/transport.py:98 |
| Whitelist disarms both org guards | Noted at the setting and at custom_auth_middleware.py:59 — the session guard my prose never mentioned |
| No logging on rejection branches | WARNING on both, plus the cross-tenant 403, with a truncated SHA-256 fingerprint rather than the token |
Left open deliberately — three threads, all needing your call rather than more code from me:
- The cloud 402 divergence. Cross-repo; I will raise the
unstract-cloudPR adding"unstract/whoami/"toSUBSCRIPTION_WHITELISTED_PATHS_LIST. You both reached the same remedy independently. request.organization_from_urlinstead of a falsyorganization_id. I think Chandru is right that it is the better design, but it changes the condition on a shared security control for every authenticated route. That wants its own PR and its own reviewers.ScopedRateThrottleand collapsing the two 401 bodies. The logging half is done. The throttle needs a rate nobody has chosen, on the endpointconfig doctor --probecalls; and collapsing the messages changes behaviour for every Bearer route. Both are decisions, not omissions.
On evidence: @hari-kuriakose noted his review collected 17 whoami tests and ran 0, so no wire behaviour was exercised. I stood up Postgres and ran them — 58 pass, ruff 0.3.4 check and format clean. The two guards I claim above are mutation-checked, not asserted.
Greptile then caught a defect in my own fix: the new logging trusted X-Forwarded-For, letting the party being logged choose the address recorded about them. Fixed in 42b695b9f — REMOTE_ADDR only, matching the two other places in this codebase that log a caller address.
I also corrected the PR description, which repeated the spec-gate claim.
Co-authored with Claude Code.
praveen-formido
added a commit
that referenced
this pull request
Sep 9, 2026
…bout them Greptile, on PR #2269. The rejection logging added in 0e9a23a took the first `X-Forwarded-For` value as the caller address. Nothing in this project validates a forwarding chain, so that value is supplied by the party being logged -- which defeats the only purpose those lines have, and puts an unvalidated header into the log stream. `REMOTE_ADDR` only now, matching `internal_api_auth.py` and `internal_base_urls.py`, the two other places that log a caller address. Behind a proxy this records the proxy; recovering the real client needs a trusted-proxy setting this project does not have, and guessing a hop count here would be the same mistake in a different shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
praveen-formido
force-pushed
the
UN-4016-platform-key-whoami
branch
from
September 9, 2026 09:50
42b695b to
9bad32a
Compare
Contributor
Author
|
recheck |
hari-kuriakose
self-requested a review
September 9, 2026 10:55
hari-kuriakose
approved these changes
Sep 9, 2026
hari-kuriakose
left a comment
hari-kuriakose
left a comment
Contributor
There was a problem hiding this comment.
@praveen-formido LGTM overall.
chandrasekharan-zipstack
pushed a commit
that referenced
this pull request
Sep 10, 2026
…endpoint sends Iteration 1 of unstract:remediation against PR #2269. Fifteen of sixteen findings; F6 is handed back (see below). The two High findings were both invisible from inside the change: F1 — the spec published `ErrorResponse` for 401/403, but `whoami` is deliberately not whitelisted, so `CustomAuthMiddleware` answers every rejection itself with a bare `{"message": ...}` and DRF's exception handler is never reached. A generated client branching on `errors[0].code` would raise on the most common failure the endpoint has. Now declares a `PlatformKeyError` shape that matches the wire, with the 500 carrying no body schema because a Django HTML 500 has none. F2 — `@pytest.mark.critical_path("platform-key-whoami")` named an id in no registry, and `tests/rig/cli.py` sets `overall_exit = 1` on an unknown marker. The rig would have failed the build on every run. CI never caught it because `test` skips on draft PRs. Now registered. Also fixed: the view returned 403 where the spec said 401 (DRF coerces `NotAuthenticated` unless the first authenticator offers a WWW-Authenticate header, and SessionAuthentication offers none) — it now returns 401 explicitly, and the branch has a test, which it never had because every other rejection is answered before the view runs. The whitelist regex is anchored, so an organisation named `whoami` no longer has its whole API treated as organisation-less. The organisation-scoped alias `/<org>/whoami/`, which the mount comment wrongly claimed could not exist, is now documented and tested in both directions. The dominant defect class was not any single bug: seven of sixteen findings were confidently-worded comments asserting mechanisms the code does not implement — that DRF resolves no user, that the org FK is non-null by construction, that no rewrite can produce `whoami/`, that a test observes middleware it never invokes. Each is corrected to what the code does, or deleted. Every fix is mutation-checked: reverting it fails a named test. That check also caught one of the new tests passing vacuously — it matched an error string the OpenAPI-validity gate produces, so it went green with the gate it was written for removed. F6 is not fixed here and needs a decision: setting `organization_id = None` makes `whoami` bypass `SubscriptionMiddleware` in the enterprise tree, where every other org-less path has an explicit `SUBSCRIPTION_WHITELISTED_PATHS_LIST` entry. The fix belongs in unstract-cloud and the intent is a product call. Findings: F1 F2 F3 F4 F5 F7(partial) F8 F9 F10 F11 F12 F13 F14 F15 F16 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
chandrasekharan-zipstack
pushed a commit
that referenced
this pull request
Sep 10, 2026
…bout them Greptile, on PR #2269. The rejection logging added in 0e9a23a took the first `X-Forwarded-For` value as the caller address. Nothing in this project validates a forwarding chain, so that value is supplied by the party being logged -- which defeats the only purpose those lines have, and puts an unvalidated header into the log stream. `REMOTE_ADDR` only now, matching `internal_api_auth.py` and `internal_base_urls.py`, the two other places that log a caller address. Behind a proxy this records the proxy; recovering the real client needs a trusted-proxy setting this project does not have, and guessing a hop count here would be the same mistake in a different shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
chandrasekharan-zipstack
force-pushed
the
UN-4016-platform-key-whoami
branch
from
September 10, 2026 05:07
9bad32a to
3a0da6a
Compare
… whoami endpoint
A caller holding a platform API key knows the secret but not what it is scoped
to: `organization_id` is only discoverable by reading it out of a web-app URL,
and every organisation-scoped endpoint takes it as a path segment. This adds
`GET /api/v1/unstract/whoami/`, which returns `{organization_id,
organization_name, permission, key_name}` read off the key row itself.
The route carries no organisation segment, and that is what made it more than a
view. `OrganizationMiddleware` matches `^/api/v1/unstract/<org>/`, so it parsed
`whoami` as the organisation and rewrote the path to a 404. Its whitelist escape
hatch then returned without ever setting `request.organization_id`, which
`CustomAuthMiddleware` reads by bare attribute access -- a 500 rather than a
skip. Both are fixed: the path is whitelisted for the organisation middleware
only (it still authenticates), and the whitelist branch now sets the attribute,
which protects any future organisation-less path.
The endpoint also flows through the committed OpenAPI spec, which needed three
gates widened: the published-prefix check now accepts the tenant mount as well
as the deployment one, `SPEC_URLCONFS` gains the new urlconf, and the two spec
tests that looped over every operation asserting deployment-specific facts now
pin those facts to the deployment operations and check only genuinely universal
ones globally. The regenerated spec is +160/-0 -- the deployment contract is
byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
…endpoint sends Iteration 1 of unstract:remediation against PR #2269. Fifteen of sixteen findings; F6 is handed back (see below). The two High findings were both invisible from inside the change: F1 — the spec published `ErrorResponse` for 401/403, but `whoami` is deliberately not whitelisted, so `CustomAuthMiddleware` answers every rejection itself with a bare `{"message": ...}` and DRF's exception handler is never reached. A generated client branching on `errors[0].code` would raise on the most common failure the endpoint has. Now declares a `PlatformKeyError` shape that matches the wire, with the 500 carrying no body schema because a Django HTML 500 has none. F2 — `@pytest.mark.critical_path("platform-key-whoami")` named an id in no registry, and `tests/rig/cli.py` sets `overall_exit = 1` on an unknown marker. The rig would have failed the build on every run. CI never caught it because `test` skips on draft PRs. Now registered. Also fixed: the view returned 403 where the spec said 401 (DRF coerces `NotAuthenticated` unless the first authenticator offers a WWW-Authenticate header, and SessionAuthentication offers none) — it now returns 401 explicitly, and the branch has a test, which it never had because every other rejection is answered before the view runs. The whitelist regex is anchored, so an organisation named `whoami` no longer has its whole API treated as organisation-less. The organisation-scoped alias `/<org>/whoami/`, which the mount comment wrongly claimed could not exist, is now documented and tested in both directions. The dominant defect class was not any single bug: seven of sixteen findings were confidently-worded comments asserting mechanisms the code does not implement — that DRF resolves no user, that the org FK is non-null by construction, that no rewrite can produce `whoami/`, that a test observes middleware it never invokes. Each is corrected to what the code does, or deleted. Every fix is mutation-checked: reverting it fails a named test. That check also caught one of the new tests passing vacuously — it matched an error string the OpenAPI-validity gate produces, so it went green with the gate it was written for removed. F6 is not fixed here and needs a decision: setting `organization_id = None` makes `whoami` bypass `SubscriptionMiddleware` in the enterprise tree, where every other org-less path has an explicit `SUBSCRIPTION_WHITELISTED_PATHS_LIST` entry. The fix belongs in unstract-cloud and the intent is a product call. Findings: F1 F2 F3 F4 F5 F7(partial) F8 F9 F10 F11 F12 F13 F14 F15 F16 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
Iteration 2. The adversarial verifier re-found F1: iteration 1 changed the
401/403 `$ref` to `PlatformKeyError` but left the `examples` block beside it
showing the handler's `{type, errors[]}` body, so the artifact contradicted
itself in a single media-type object -- schema requiring `message`, example
showing `errors[0].code`. A client author reads the example, not the `$ref`, so
the defect F1 was filed against survived its own fix.
Cause: `drf_standardized_errors` appends an example of the handler body to
every 4xx/5xx keyed on the status code alone, never consulting the declared
serializer (`openapi.py:343-356`), so overriding `responses` was never going to
be enough. `PlatformKeyAutoSchema` suppresses that injection for this view
only; the deployment operations keep their examples, which are correct there
because those really do return the handler body.
Also fixed, all four raised by the same verifier against iteration 1's own work:
the module docstring claimed the middleware answers *every* rejection, which the
401 iteration 1 added to the view had just made false; the new spec test lacked
the `assert reads` guard that the same commit added twelve lines below it, so it
passed vacuously on an operationId rename; the alias test's docstring claimed
mount-order coverage a mutation disproved (swapping the mount leaves every test
green); and the 403 body was declared but asserted nowhere, which is the exact
"status asserted, body unasserted" gap that let the original spec lie.
`test_no_published_example_contradicts_its_own_schema` now pins the property
structurally for every operation. It found three pre-existing instances in the
merged deployment spec -- `status` 406/500 and `execute` 500 all declare a
non-error body beside a handler-shaped example. Those are recorded in
`_KNOWN_EXAMPLE_DIVERGENCES` rather than fixed here: they predate this branch
(verified against c49968e) and belong to whoever owns that spec. The check
fails on any new instance.
Findings: F1(re-fixed) N2 N3 N4 N5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
…claiming it cannot
Iteration 3. The verifier re-found N2 with a counter-example rounds 1 and 2
both missed: `POST /api/v1/unstract/whoami/` with a `read_write` key -- the
model default -- is passed by the middleware, refused by DRF, and comes back
405 in the exception handler's `{type, errors[]}`. Four comments across three
files asserted that shape was unreachable here, and that claim was the stated
justification for both not reusing `ErrorResponse` and suppressing every error
example on the view.
So the endpoint has always returned two error shapes and the spec described
one. 405 is now declared with `ErrorResponse`, and `PlatformKeyAutoSchema`
narrows its suppression to the statuses the middleware actually answers (401,
403) rather than blanket-stripping, so the 405 keeps the handler example that
is correct for it. Verified on the wire: a `read_write` key POSTing gets 405
`{type, errors[]}`; a `read` key gets 403 `{message}` from the middleware
instead. Both are now pinned, against each other, so neither path can look like
the only one.
The absolute clauses are deleted rather than rewritten a third time. Two
rounds of rewriting them produced two more false statements; what remains says
only what is enforced.
Also from the same verifier: `_KNOWN_EXAMPLE_DIVERGENCES` was keyed on
`(operationId, code)`, which exempted those coordinates forever -- a new
divergent example injected at `status` 406 passed. Now keyed on the example
name too, so the three recorded rows stay exempt and a fourth fails; the
comment claiming the check catches new instances is true as written for the
first time. Dropped a dead exemption branch that could never match, and a
comment misattributing which commit added a guard and how far below it sits.
Findings: N2(re-fixed) + 4 new from verifier round 2
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
for more information, see https://pre-commit.ci
…not return Iteration 4, and the last: N2 is escalated rather than attempted a fourth time. Round 3 declared 405 under the `get` operation. A verifier's 42-cell matrix (3 tiers x 7 methods x 2 routes) shows GET returns 200, 401 or 403 and never 405 -- so the declaration was unreachable at the coordinate it was written, and the 405s that do occur (POST/PUT/PATCH at read_write+, DELETE at full_access) have no operation in the spec to attach to. The kept example made it worse: it published `Method "get" not allowed.` under the GET operation, a hardcoded placeholder from drf_standardized_errors, not derived from the route. The declaration is withdrawn. The behaviour is real and is now stated in the operation description in prose, which is where a fact about methods the route does not serve can honestly live. Also corrected, all introduced by round 3 and all found by the same verifier: an I001 failure of the repo's PINNED ruff 0.3.4 hook, which round 3 introduced and the installed ruff 0.15 does not report -- verified clean at 7eb41bc and failing at 32ec142, then confirmed the fix satisfies both versions; a comment asserting everything else "keeps the handler shape", contradicted by the 500 declared eight lines below it; a true clause round 3 deleted while rewriting the sentence around it, restored verbatim; and a test whose name claimed a relationship to the spec that nothing in it tested -- renamed to what it actually asserts, with the gap it does not cover named in its docstring. WHAT IS ESCALATED, and why this loop stops here: N2 has survived three fix attempts. Each attempt was a correct reading of the previous failure and each introduced a new defect: round 2 rewrote a false claim into a stronger false one, round 3 fixed the claim and declared the status in the wrong place. Three consecutive rounds introducing new defects is the convergence tripwire, and the discriminator applies -- the fixes keep needing an exception to a shared rule, which means the rule is wrong, not the wording. The rule that is wrong: this path serves GET, but answers non-GET methods with a status and a body shape that no `get` operation can describe. OpenAPI attaches responses to operations, not paths. Options, none of which a remediation loop should pick unilaterally: declare stub operations for the methods purely to carry a 405; accept prose (what this commit does); or change the view so every method the tier permits is answered in one shape. Findings: NEW-1..NEW-5 from verifier round 3 fixed; N2 ESCALATE (attempts=3) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM
**Spec gate (the change request).** `PUBLISHED_PATH_PREFIXES` carried
`api/v1/unstract` -- exactly `TENANT_SUBFOLDER_PREFIX` -- and the check is a
`startswith` over the union, so an `API_DEPLOYMENT_PATH_PREFIX` pointed anywhere
under the tenant mount passed a gate whose own comment says it exists so such an
override fails. Each entry is now the route rather than the mount it hangs off.
Verified: with `API_DEPLOYMENT_PATH_PREFIX=api/v1/unstract/deploy` the generator
now fails; unset, it regenerates byte-identically.
**403 withdrawn from the published operation.** Every route to one is closed on
`/api/v1/unstract/whoami/`: the belongs-to-org guard is skipped, `allows` admits
GET at every tier, and an unknown tier is barred by migration 0003's check
constraint. It moves to the deployment-specific assertion alongside 400/404,
where 403 is genuinely reachable; the universal set becomes {401, 500}. This
resolves the contradiction between that universal rule and the dead-branch rule
the same suite states.
**Middleware ordering is now asserted against the shipped setting.** The suite
pins MIDDLEWARE, which hid the one ordering this route depends on -- reverse
TENANT and CUSTOM_AUTH in settings and all 17 tests stayed green while every
whoami request would AttributeError in production. Verified by doing exactly
that: only the new assertion fails.
Also corrected: the docstring claiming the operation publishes two error shapes
(it publishes one, and `ErrorResponse` is not even imported); "one shape covers
every rejection", which the declared 500 contradicts; the whitelist entry
re-spelling a mount the file already derives, now built from
`TENANT_SUBFOLDER_PREFIX` with `re.escape` since `PATH_PREFIX` is environment
data landing in a regex; and the false claim that cloud test settings drop
`CustomAuthMiddleware` -- `unstract-cloud`'s `test_cloud.py` says the opposite in
as many words.
Added: `WWW-Authenticate` on the 401 (RFC 9110 s11.6.1), spelled as in
`mcp_server/transport.py`; WARNING logs on the two credential-rejection branches
and the cross-tenant 403, with a truncated SHA-256 fingerprint rather than the
token, so "was this key ever presented?" has data to answer from; a note that
MCP's `organization` holds an id, not a name, so nobody aligns the two surfaces
by field name and ships an id where a display name belongs; and a note in
settings that an entry in this whitelist disarms both organisation guards.
The example-divergence allow-list is now keyed by schema name and asserts its
entries still match, so a fixed divergence fails until its entry is deleted --
which immediately caught three wrong schema names in my own first draft of it.
Its check ran zero assertions against the committed spec, so a positive case now
proves it fires.
58 tests pass against live Postgres, ruff 0.3.4 check and format clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
@hari-kuriakose asked for this specifically: the PR's prose reasoned carefully about the Bearer branch's belongs-to-org check and never mentioned that the session branch's org-access-denied check is conditioned on the same attribute, so a path added to ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS loses both. The next reader looks here, not at the settings file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
…bout them Greptile, on PR #2269. The rejection logging added in 0e9a23a took the first `X-Forwarded-For` value as the caller address. Nothing in this project validates a forwarding chain, so that value is supplied by the party being logged -- which defeats the only purpose those lines have, and puts an unvalidated header into the log stream. `REMOTE_ADDR` only now, matching `internal_api_auth.py` and `internal_base_urls.py`, the two other places that log a caller address. Behind a proxy this records the proxy; recovering the real client needs a trusted-proxy setting this project does not have, and guessing a hop count here would be the same mistake in a different shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ
chandrasekharan-zipstack
force-pushed
the
UN-4016-platform-key-whoami
branch
from
September 10, 2026 06:35
3a0da6a to
bf8bfaf
Compare
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
chandrasekharan-zipstack
added a commit
to Zipstack/unstract-cli
that referenced
this pull request
Sep 16, 2026
…`deployment ls` (#3) * UN-4016 [FEAT] Support a platform API key: `auth whoami` and `deployment 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 * UN-4016 [FIX] Send the platform key where the caller said, not where 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 * UN-4016 [FIX] Stop the fixes from re-breaking what they fixed 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 * UN-4016 [FIX] Resolve the platform host by tier, not by product 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 * UN-4016 [FEAT] Use the released SDK's platform client, not a hand-built one `unstract-client` 1.7.0 publishes both platform-key operations, so the CLI stops reaching into `unstract.clone` -- the org-cloning tool's hand-written admin client -- and drops `CLIPlatformClient` and its hand-built whoami URL entirely. Per `bump-client-pins`: the pin moves to 1.7.0, the lockfile is regenerated, the vendored spec is re-synced byte-for-byte from the spec committed in the client at tag `v1.7.0`, and `provenance.json` records the upstream revision that tag's `gen_sdk.sh` names. I verified the chain rather than assuming it -- the tag's recorded sha256 and the spec committed there agree, and both match what is now vendored. **A regression this swap introduced, caught before it shipped.** `PlatformClientError` derives from `APIDeploymentsClientException`, whose arm in `translated()` maps everything to USAGE, and the released client carries no status code -- `_read_or_raise` raises with the status in prose only. So a rejected platform key exited 2 instead of 3, contradicting the README's exit-code table. The suite did not notice because the fake client still raised the old `PlatformAPIError`: green tests over a path the real client no longer takes. Both are fixed, and the status-to-exit-code mapping is now pinned for 401/403/404/429/500 plus the unparseable case, and verified live against a server returning each. Recovering the status from the message is a bridge, not a design. Asked upstream to carry `status_code` on `PlatformClientError`; the parse should be deleted when that lands rather than kept as a fallback. **The listing is paginated now.** `list_deployments` returns `{count, next, previous, results}` where `list_api_deployments` returned a flat list. `ls` reports `shown` and `count` separately with a `more` flag, because this command does not follow pages and reporting one number as the other would tell a caller with more deployments than a page that they had seen everything. **`platform.api_prefix` is removed.** The generated builders bake the spec's paths and `PlatformKeyClient` discards any path on `base_url`, so the setting could no longer take effect. Worth knowing: a self-hosted install that remounts `PATH_PREFIX` is not reachable through the generated client at all. `clone` is unaffected -- its `--api-prefix` is its own flag, not this setting. 283 tests pass, ruff clean. Both operations exercised live against a throwaway server: whoami reaches `/api/v1/unstract/whoami/` and the listing `/api/v1/unstract/<org>/api/deployment/`, both bearing the platform key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7U4ggFchu91zKYcRxFRNZ * UN-4016 [REFACTOR] Flatten the platform key into the docstudio block The platform API is served by the same deployment as the API deployments it manages, so a separate `[profiles.X.platform]` product only ever duplicated docstudio's host. The key now lives on the docstudio block as `platform_key`, beside `api_key` and `org_id`; `$UNSTRACT_PLATFORM_KEY` is unchanged and still outranks the file. - Delete the cross-product tier walking that existed only to share `base_url` between the two blocks. - Treat `platform_key` as a credential everywhere `api_key` is one: scrubbing, the shell-history warning, and the untrusted-project filter. - `auth` and `docstudio` groups take `--platform-key`; `auth --api-key` is gone, it named the wrong key. - `config init` no longer writes a platform block or a deployments stub. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FEATURE] Key a deployment entry by its API name, and say which key a run used A `[profiles.X.deployments.<api_name>]` entry now holds nothing but the `api_key` that runs that one deployment. The alias layer -- an entry naming its own `api_name`, `org_id` and `api_key`, addressed by a local nickname -- is gone: `deployment run` and `status` take the API name directly, as `deployment ls` prints it, and most profiles need no deployments section at all. The key for a run resolves flag > env > per-deployment entry > profile key. The entry is the most specific value within the profile tier, not a tier of its own, so `$UNSTRACT_DEPLOYMENT_KEY` still wins over a file value. Pinned by an ablation test that sets all four sources and asserts the winner as each is removed. Nothing resolving is a usage error raised before any request, listing every place the CLI looked and the exact command for each remedy. A 401 from the server is translated to name the deployment and the `config set docstudio api_key <key> --deployment <api_name>` line that gives it a key of its own -- the hint that was previously unreachable. A 404 points at `deployment ls`. `config set` gains `--deployment API_NAME` as the write path for a per-deployment key. `config doctor --probe` cross-checks deployment entries against the live listing when a platform key is available and reports orphaned entries as warnings, never as failures; without a key the check is skipped silently, and without `--probe` doctor stays offline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FEATURE] Add `auth login` to store keys in a profile, checking each one it can One command for the first-run: at a terminal it asks for the platform, deployment and LLMWhisperer keys in turn, one hidden and skippable prompt each, with at least one required. Without a terminal it takes the same three as `--platform-key`, `--deployment-key` and `--llmwhisperer-key`, any one of them as `-` to read from stdin, and asks nothing; a bare non-TTY run fails fast naming the flags. The platform key is checked with `whoami` and the organisation it resolves is stored beside it; the LLMWhisperer key is checked against the usage endpoint; a deployment key has no side-effect-free endpoint, so it is stored as given and the result says so. Every check runs before the one write, and the keys land as literals in the 0600 config file. A re-run replaces the keys given and keeps the rest, so rotation is the same command. A profile that already belongs to another organisation is not repointed silently: at a terminal the command offers a new profile named after the organisation, or overwrites on request; without one it fails naming both organisations and `--force`. Validation resolves through the profile being written, so a new profile is checked against the flags and the environment rather than borrowing the default profile's host. `whoami`'s profile writer is split into the pieces `login` shares -- the project-local refusal and the profile-selection ladder -- rather than duplicated. `config init` stays the skeleton writer and now points new users at `auth login`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [DOCS] Lead the README with `auth login` and describe each credential once The quickstart opens with `auth login` for a person at a terminal and an environment-only three-liner for an agent or CI. The three keys are described once, by the job each does and where it is minted, in place of the per-command capability lists that repeated one another. The `[profiles.X.platform]` block and the alias examples are gone with the features; the config example shows the flattened `platform_key` and a per-deployment entry as the exception it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FIX] Say why an explicit `doctor --probe` skipped the deployment cross-check Without a platform key the entries cannot be checked against the organisation, and skipping silently under an explicit flag leaves the caller wondering why nothing happened. A bare `doctor` stays silent. The not-found hint now also names `doctor --probe` as the way to find entries the organisation no longer has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FIX] Keep the verified host on a new login profile, confirm a colliding name, fail a failed probe Three review findings on `auth login` and `doctor --probe`. A profile `login` writes now records the base URL the key was checked against whenever it has none of its own (or a flag named one): the profile the org-mismatch guard creates inherits the host of the profile the login started from, and a brand-new profile records the host it was verified on rather than falling back to the built-in default later. A profile name typed at the guard's prompt that already belongs to a third organisation is confirmed separately, and declining re-prompts, so the guard's own remedy cannot perform the overwrite it exists to prevent. A deployment listing that fails during `doctor --probe` is now a failed check -- reported under `problems` with a non-zero exit -- rather than being swallowed and read as "no stale entries". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FIX] Store the host a key was checked against when the guard changes profiles The organisation-mismatch guard lets the user pick an existing profile as the destination. That profile's own base_url was kept even though the key had been validated against the profile the login started from, so the key could be persisted next to a host it was never checked on. When the final profile differs from the one the run resolved, the validated host now replaces whatever the destination recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FIX] Replace, rather than merge into, the profile the login guard names The organisation-mismatch guard lets the user name an existing profile as the destination. Its host was already being replaced with the one this key was checked against, but credentials this login did not supply -- a stored deployment key, per-deployment entries -- stayed behind and would have been sent to that host. The named profile is now rebuilt from what this login verified, and any existing name is confirmed before it is replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2SKeanczq13ngDQNbgFNL * UN-4016 [FIX] Report a platform client the settings cannot build as a usage error The client validates the host and the key in its constructor, and that failure reached no arm of the entry point: a base URL without a scheme, or a blank key, produced a traceback on stderr, nothing on stdout and exit 1 -- where the CLI promises an envelope. Every command that builds the client was affected, including `config doctor --probe`, which crashed after reporting. Refused at the factory, beside the timeout guard that does the same, so all call sites are covered whether or not they build inside `translated()`. Also assert the organisation the listing is actually made with: the factory parameter the test pinned is accepted and ignored, so a cross-tenant listing would have shipped green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Give --transport-timeout one meaning and a bound that exists The help promised a 60s client default that the client does not apply: unset meant unbounded, so a black-holed host hung forever with no output. The same flag also read differently on the two groups -- 0 was unbounded on one and a usage error on the other. Both groups now take the same type and default, 0 waits forever on both, and a negative value is refused at the flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Report 406 as a consumed result only on the reads that serve one A 406 was mapped to ALREADY_CONSUMED for every endpoint. Only the deployment status read and `whisper retrieve` hand their result over exactly once; elsewhere a 406 is content negotiation, and telling the caller their result was already retrieved sends them after a command they never ran. exit_code_for_status, error_from_status, hint_for, translated, translating and raise_for_result take a one_shot flag; the whisper retrieve and deployment status call sites pass it. Every other 406 now exits GENERIC with a hint that names base_url and proxies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Stop re-sending a rejected platform key for the entry check in doctor `config doctor --probe` checked deployment entries against the organisation whenever a platform key resolved, including when whoami had just refused that key. The listing could then only fail the same way, and the same refusal was reported twice in `problems`. The entry check now runs only when the platform probe passed; when it did not, the skip is said in a note, as it already is when no key resolves at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Register the keys `auth login` is given so a later error cannot print one The scrubber only knows a credential the config layer resolved. `auth login` takes its keys from flags, stdin or prompts and writes them straight out; the deployment key is never sent anywhere at all, so none of them was registered and a failure quoting one back would print it in full. Every key given is registered as soon as it is collected, before the first call that could echo it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Say when a login stored a platform key that resolved no organisation `auth whoami` warns when the identity carries no `organization_id`; `auth login` wrote the key and reported success, leaving the profile without the one field every docstudio command needs and nothing on stderr to say so. Both now report the same answer the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Treat a deployment listing that is not a list as a protocol failure Both readers of the listing took `results` as given: `deployment ls` projected its fields, which raises AttributeError on anything but rows, and `config doctor --probe` read a truthy non-list as "the deployment is gone" and named every entry stale. Either way a proxy or web app answering in the API's place came back as a traceback or as wrong advice. `deployment_rows` now checks the shape once for both and raises a server error whose hint names `base_url`, carrying the body in `details`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [TEST] Cover four behaviours nothing failed on Each of these could be deleted with the suite still green: - the deployment entry's own key being registered for scrubbing, which is resolved on a path of its own and so is not covered by the profile key's test - `auth whoami` reporting an identity that carries no organization_id - `auth login` exiting SAVE_FAILED rather than a generic failure when the keys were accepted and only the write failed - `auth login` moving an existing profile to the host a --base-url flag had the key checked against Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [CHORE] Delete a config accessor nothing calls `get_explicit` has had no caller since it was written; the distinction it draws between a named default and an unset value is documented in a docstring nobody reads from a call site. `_explicit`, which it wrapped, is still what `_resolve` uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Catch the non-JSON body the clients actually raise, and drop an arm nothing reaches The translator caught `requests.exceptions.JSONDecodeError`, which none of these clients raise: both facades swallow a bad body and re-raise their own error, and the LLMWhisperer client parses with `json.loads`, whose `json.JSONDecodeError` is the base class rather than the subclass. A web-app or proxy host answering 200 with HTML therefore reached the entry point as a crash. Caught on the base class it is a server failure with a hint naming `base_url`. The `PlatformAPIError` arm is unreachable for the same kind of reason -- only the clone orchestrator raises it, and `clone` catches it itself, outside any translator. Deleted, with the body-truncation it carried moved to the arm that does run: `error.message` is published as a one-line summary and that exception appends up to 2KB of response body to its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [DOC] Correct comments that name the wrong cause or recount how the code got here - `platform_cmd` claimed no OpenAPI spec is vendored for the platform API; both operations are in the vendored docstudio spec. What is true is that these commands declare their flags by hand. - The transport timeout is not a socket timeout, and it is not the deployment client's alone. - The status parsed out of a platform failure is explained by what the client sends today, without the upstream request and the plan for the day it lands. - Three comments and two test docstrings recounted what an earlier version did or what a past bug cost; each now says what the code is for, which is what reads correctly to someone who never saw the earlier version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Read the configured organisation without the whitespace around it An org_id is copied out of a web-app URL or a console, and comes with whatever was selected. It goes straight into the deployment URL, so a trailing space turns into a 404, or into a name no listing shows. It is trimmed on the way out, and a value that is only whitespace is the same "no organisation is configured" as an empty one. The same commit corrects two comments in this module: the org_id parameter is not passed at every call site, and the timeout httpx refuses is a negative one, at send time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [DOC] Correct four things the README says about flags, init and --probe - The docstudio group's connection flags were listed as `--base-url` and `--org-id`; it also takes `--api-key` and `--platform-key`, and the auth group takes `--base-url` as well as `--platform-key`. - `deployment ls` authenticates with the platform key and refuses `--api-key`, which was not said anywhere. - `config init` writes a `cloud-eu` profile too. - `--probe` was described as checking "the keys"; it checks the two that have something side-effect-free to call, which is the same pair `auth login` checks and not the deployment key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Name the key a deployment rejected where it was read from A 401 on a deployment always advised storing a key for that deployment alone. When the rejected key *was* that stored key, the advice names the step that has already failed; when it came from --api-key or the environment, editing the profile changes nothing until it is unset, because both outrank it. The resolver now reports the tier alongside the key -- one ladder, so the two cannot disagree -- and the hint says which of the four was rejected. The profile-key case keeps the wording it had. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [DOC] Present both ways of storing a credential instead of preferring one The README and `config set --help` told the reader to prefer `env:VAR_NAME` while `auth login` -- the first command either of them documents -- writes keys literally by design, into a file created 0600. Read together they say the wizard does the wrong thing. Both forms are now described for what each is good for: indirection keeps the secret out of a file that is shared or committed, literal storage is what the wizard writes and what it validates at write time. The `config set` warning still says a literal was stored, without the blanket advice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Store the host a login checked against when the environment chose it `auth login` replaced a profile's `base_url` only when a --base-url flag had been passed. A host coming from the environment was used for the check and then dropped, so the profile kept whatever host it already held and the key was stored beside a server it had never been checked against. The decision now follows where the host was actually resolved from, so a flag and an environment variable are persisted alike. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Say when the host a login checked against came from a reference A profile may record `base_url = "env:VAR"` rather than a host, which is a deliberate choice that resolution stays mutable; login leaves it alone. The keys are then stored beside a host that can change without the file changing, and nothing said so. Login now reports the variable and the host it actually checked against once per product, leaving the stored reference as the user wrote it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * chore: trim comments that narrate rather than state a constraint The long ones restated what the next line does, carried detail that goes stale (library versions, exact counts) or explained a decision at more length than the code it sits above. Each is now one or two lines saying why, readable without knowing how the code got there. Behaviour is untouched: every changed file parses to the same AST once docstrings are stripped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Name the page the org-wide deployment key is actually minted on The CLI and the README both sent a caller to Settings -> API Key Manager for a key covering every deployment in the organisation. It is minted under Settings -> Platform -> Global API Deployment Keys, which is also what the documentation says. The platform key is a separate page and is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Spell the deployment key's page as the navigation labels it The page sits directly under Settings, not behind a Platform submenu, so the path named in the CLI and the README now matches what a caller reads on the screen. The test pins the whole path rather than the page name, so a segment cannot drift back in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * chore: trim comments that narrate rather than state a constraint The note on the platform error arm quoted a body-truncation size that only holds until the client changes it. The constraint it exists for -- the body is appended to the message, and the summary published to the caller is one line -- does not depend on the size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * chore: drop comments that argue the design rather than guard an edit A comment earns its place by stopping a plausible edit from being wrong: an ordering requirement, a scrub bypass, a one-shot read, a trap in a library. Comments that only justify a decision the code already makes are removed, and constraint statements padded with argument are cut to the constraint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Stop a login that moves a profile's host from keeping the keys it did not check A login stores the host its keys were checked against. Supplying only one key while a flag or the environment selects another host left every other key in the profile -- the deployment key, each per-deployment entry, the LLMWhisperer key -- sitting beside a host none of them had been accepted by, and the next command sent them there. Keys the run did not supply are now dropped when the host it stores differs from the one the profile resolved on its own. At a terminal the question names them and declining writes nothing; without one the run fails on USAGE until `--force`, which keeps the meaning it already had: the replacement is accepted. A re-login against the same host is untouched, so rotation still keeps the rest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Say that the organisation-wide deployment key needs an admin too The menu entry that mints it is shown only to organisation admins, and the same paragraph already says so for the platform key. A non-admin following the hint looks for a menu item that is not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Compare hosts by what they name, not by how they were typed The stranding check compared `base_url` strings as typed, so a login against `https://stored.example` fired against a profile holding `https://stored.example/`, and asked to drop keys that were going nowhere. Scheme and host are now lowercased and a trailing slash ignored at compare time only; the profile keeps the URL the user typed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Remove a deployment entry with its key rather than leave it empty Dropping a deployment key left `[profiles.X.deployments.Y]` behind as an empty table, and an entry with no key still counts as a deployment the profile holds: `deployment ls` and `config doctor` kept naming it. The entry now goes with its key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ * UN-4016 [FIX] Read the host a profile held from the file, not through the ladder The stranding check read the profile's prior host through `ResolvedConfig`, where the environment outranks the file. With `UNSTRACT_BASE_URL` set, both sides of the comparison resolved to the environment's host, the check saw no change, and login stored the new host while keeping keys checked against the old one. The prior host now comes from the profile block itself: a literal, an `env:` reference resolved, or the built-in default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chandrasekharan M <chandrasekharan@zipstack.com> Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
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
4 participants
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
GET /api/v1/unstract/whoami/— an organisation-less endpoint that authenticates with a platform API key and answers{organization_id, organization_name, permission, key_name}, read off the key row itself. It reaches the generated OpenAPI spec, so the published client and the CLI pick it up.Builds on the spec pipeline from #2237 (UN-4009), which merged on 31 Aug — this was written against that branch and has been rebased onto
mainsince, with the spec regenerated and re-verified against the merged result.Why
Before a caller can use any organisation-scoped endpoint they need
org_id, and it has no documented source — it is the first path segment of every web-app URL (useMainAppRoutes.js,<Route path=":orgName">) and nothing says so. A platform key already carries its organisation by construction:PlatformApiKeyinheritsDefaultOrganizationMixin, whose FK is stamped at mint time from the minting admin's active org, andkeyis globally unique, so a bearer token maps to exactly one row and therefore one organisation. Nothing exposed that association over HTTP.Every comparable CLI that predates OAuth (
doctl,twilio,sentry-cli) issues one opaque token that carries its own scope and is validated the moment it is supplied. This is the endpoint that lets ours do the same.How
platform_api/whoami_views.py— a field read.permission_classes = []becauseCustomAuthMiddlewarehas already resolved the token to a key row, bound the service account torequest.userand enforced the tier against the method; a permission class would re-ask a question already answered.authentication_classesis deliberately not set — the project configures noDEFAULT_AUTHENTICATION_CLASSESthat resolves a user, so DRF's default returnsNoneand the middleware'srequest.usersurvives into the view. No extra query:custom_auth_middleware.pyalreadyselect_relatedsorganization.platform_api/whoami_urls.py+ a mount inbase_urls.py— its own module because the spec's urlconf selector can only pick out a mount declared with a dotted module path (the tenant and public mounts pass a list, sourlconf_name.__name__isNonefor them). Mounted ahead of the tenant urlconf so resolution is deterministic rather than relying on fall-through; organisation-scoped paths are rewritten before routing and none of those rewrites can producewhoami/.OrganizationMiddlewarematches^/api/(v[12])/unstract/(?P<org_id>[^/]+)/, so/api/v1/unstract/whoami/parsedwhoamias the organisation and rewrotepath_infoto/api/v1/unstract/→ 404, withrequest.organization_id = "whoami"making the org-match guard atcustom_auth_middleware.py:94403 every valid key. The whitelist escape hatch (ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS, previously[]) returns early without ever settingrequest.organization_id, which that same line reads by bare attribute access →AttributeError→ 500. So: the path is added to that list, and the whitelist branch now sets the attribute. The second half is a latent-bug fix that protects any future organisation-less path —test_whoami.pyfails with exactly thatAttributeErrorif it is removed.This is not
WHITELISTED_PATHS: that list skips authentication entirely, which for this endpoint would mean answering with someone else's organisation or none at all. A test asserts it stays out.platform_api/openapi_schema.py— mirrorsapi_v2/openapi_schema.py: a spec-only serializer that never builds a response, withpermissionsourced fromApiKeyPermission.choicesso a new tier cannot reach the API without reaching the spec.auth=[{"platformKey": []}]is mandatory, not decorative — an operation that omits it regresses tocookieAuth/basicAuth, because DRF's unset authentication default gets introspected as a decision.api/v1/unstract— the whole tenant mount — which, against astartswithover the union, let anAPI_DEPLOYMENT_PATH_PREFIXpointed anywhere under that mount pass the gate. Caught in review by @hari-kuriakose and @chandrasekharan-zipstack and fixed in0e9a23a37; an overridden prefix now fails the gate again, which is verified rather than asserted;SPEC_URLCONFSgains the new urlconf; and the two tests that looped over every operation asserting deployment-specific facts now pin those to the deployment operations and assert only genuinely universal ones (401/403/500, and that each operation names exactly one declared bearer scheme) globally. Declaring400/404on an operation that carries no body and names no resource would hand clients a dead branch — the same sintest_only_the_execution_endpoint_...already guards against.Can this PR break any existing features. If yes, please list possible items. If no, please explain why.
The two middleware changes are the risk surface, and both are additive:
ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHSgoes from[]to one regex matching only/api/v1/unstract/whoami/. Cloud and on-prem.append()to this list rather than reassigning it, so their existing entries are unaffected.request.organization_id = Nonein the whitelist branch can only turn anAttributeErrorinto the skip the earlyreturnalready intended. No path reached that branch before this PR, since the list was empty.The new URL mount sits ahead of the tenant urlconf but cannot shadow it:
OrganizationMiddlewarerewrites every organisation-scoped path to/api/v1/unstract/<rest>/before routing, and no rewrite yieldswhoami/. Verified by hand against a running server —/api/v1/unstract/acme/api/deployment/still returns 200 with the same key, and a key from another organisation still gets 403 there.Cloud and on-prem inherit both the mount and the setting with no change in
unstract-cloud:cloud_base_urls.pyandonprem_base_urls.pybothfrom .base_urls import urlpatternsand append.The regenerated spec is +160/−0 — the deployment operations are byte-identical, so nothing generated from it changes.
Database Migrations
None. No model changes.
Env Config
None.
ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHSis a settings constant, not an env var, and is not intended to be overridden per installation.Relevant Docs
None in this repo.
unstract-docshas aPlatform API Keyspage that could gain awhoamientry once this lands; not blocking.Related Issues or PRs
unstract auth whoami,docstudio deployment ls); PR pending repo accessDependencies Versions
None added or changed.
Notes on Testing
46 passedin the two suites this touches (platform_api/,api_v2/tests/test_docstudio_spec.py), of which 10 + 3 subtests are new endpoint tests and 6 are new spec anchors. Full backend suite baselined against the unmodified parent commit: identical failure set, +16 passing.platform_api/tests/test_whoami.pygoes through the real URLconf and a real middleware chain, because everything that makes this endpoint work happens before the view. It covers: the organisation resolving from the key and not the URL (two organisations, one URL, two answers); every tier reading its own identity; missing/malformed/unknown/inactive keys; that the route actuallyresolve()s, so a 404 cannot masquerade as a passing 401; and that the path is not inWHITELISTED_PATHS.Also exercised by hand against a running server with two seeded organisations —
whoamianswers each key with its own organisation over the same URL, all four rejection paths return 401, and the organisation-scoped regression checks above pass.Screenshots
n/a — no user-facing surface.
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code
https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM