[pull] main from Stirling-Tools:main - #334
Merged
Merged
Conversation
## Current state
A self-hosted instance that has linked its SaaS account talks to SaaS
over two channels:
1. **Server to server.** `AccountLinkClient` uses
`java.net.http.HttpClient` with the device credential against
`/api/v1/instance/*`. No browser, so CORS never applies. This works
today.
2. **Browser to SaaS.** The portal's `apiClient.saas` fetches SaaS
directly from the instance's own page, carrying the signed-in admin's
Supabase JWT.
Channel 2 is blocked. `corsConfigurationSource()` allows a fixed origin
list (`localhost:3000/5173/8080`, `stirling.com`, `app.stirling.com`,
`api.stirling.com`, the Tauri origins, plus loopback-any-port outside
production). A customer's instance is on none of them.
## Problem
**Self-hosted origins cannot be allow-listed.** Every deployment has a
different scheme, host and port, they are not known ahead of time, and
each entry would be a standing grant to return credentialed responses to
that origin. `setAllowCredentials(true)` also rules out `*`, since
browsers reject that pair.
**Three endpoints were pointed at the wrong backend.** `fetchDocuments`,
`fetchAuditLog` and `exportAuditLog` chose their backend with
`apiClient.saas.isConfigured()`, which answers "is a SaaS URL set", not
"am I the SaaS build". Those coincide only while self-hosted never sets
`VITE_SAAS_API_URL` — which linking now requires.
## Solution
### 1. Send the instance's own data to the instance
Documents and the audit trail are local to a self-hosted deployment.
SaaS holds no audit rows for a linked instance: the daily sync carries
three counters (`api`, `ai`, `automation`) and nothing else. So a linked
instance was silently showing the admin's **cloud team** data in place
of the server's, with no error.
`apiClient.local` already resolves per flavor via the `localBackend`
seam (self-hosted → local Spring bearer, SaaS → SaaS backend + Supabase
JWT), so these three just use it. No-op on the SaaS build, correct on
self-hosted.
### 2. Remove the affordance that caused it
`apiClient.saas.isConfigured()` is deleted. With those three call sites
fixed it was dead code, and it contradicted the contract the same file
documents a few lines above: calls throw `SaasUnconfiguredError` so
callers can surface a "configure" state "rather than silently routing to
the wrong domain". Removing it makes a relapse a **compile error**
rather than a silent wrong-backend read, which is stronger than a lint
rule. The module header now states the rule directly.
Nothing replaces it: the correct pattern is already in use in
`Usage.tsx`, which catches `SaasUnconfiguredError`, and `http.test.ts`
already pins that behaviour.
### 3. Any-origin CORS for the cloud-only surface
What remains genuinely cross-origin is what has no self-hosted
equivalent: billing, procurement, and legal documents. Register a second
CORS config for those, with `allowedOrigins("*")` and
`allowCredentials(false)`, ahead of the existing `/**` entry.
`UrlBasedCorsConfigurationSource` returns the first matching pattern
rather than the most specific, so order matters; the tests pin the
behaviour either way.
| Pattern | Contents |
| --- | --- |
| `/api/v1/payg/**` | wallet, wallet/refresh, invoices, payment-method,
cap |
| `/api/v1/procurement/**` | one controller |
| `/api/v1/legal/**` | one controller |
| `/api/v1/account-link/instances/**` | the Settings page's "who is
linked" table, and revoke |
All are cloud-only, which is what makes a prefix safe: a team's roster
of linked instances spans instances, and an instance knows only itself.
Only the `instances` half of account-link is opened; the `connect/*`
handshake never reaches a browser on the customer's origin, since the
instance backend calls `request` and `claim` server-side and we serve
the approval page. Methods are limited to GET, POST, PATCH and OPTIONS;
headers to `Authorization`, `Content-Type` and `Accept`.
`/api/v1/instance/**` is deliberately excluded: server to server, should
never see a browser origin. The block sits behind the existing
`stirling.billing.account-link.enabled` flag, so a deployment not
running account linking gets no wildcard at all.
## Why the wildcard is safe here
**Only because it carries no credentials**, which holds on this chain:
- bearer-token only: `STATELESS`, no form login, no HTTP basic
- nothing in `app/saas` reads a cookie (no `@CookieValue`, no
`getCookies()`)
- the cookie/session chain, `SecurityConfiguration`, is
`@Profile("!saas")` and does not run here
- `apiClient.saas` never sets `credentials` on `fetch`, so it defaults
to `same-origin` and sends no cookies cross-origin
- the JWT is in localStorage and attached explicitly, so a hostile page
has nothing to ride on: it cannot read another origin's storage
That is the same reasoning the file already uses to justify disabling
CSRF on this chain.
**Authorisation is unchanged.** Callers still present a JWT and are
still resolved to a team by the existing gates; this decides only which
origins may read a response. In particular it does **not** let the
instance act as a user — the device credential gains no new reach, which
a backend proxy would have given it.
**First-party is unaffected.** On the SaaS build `saasApiBase()` returns
`""` (`VITE_API_BASE_URL=/`), so `app.stirling.com` calls these paths
same-origin and is exempt from CORS entirely. `allowCredentials(false)`
cannot reach it.
## How to test
```bash
ENABLE_SAAS=true ./gradlew :saas:test --tests "*SupabaseSecurityConfigMoreTest*"
```
18 cases in the new `LinkedInstanceCors` class: every remaining
`apiClient.saas` path resolves to the wildcard config and never has
`allowCredentials=true`; PATCH is permitted for the cap endpoint;
`/api/v1/instance/sync`, the three now-local ui-data paths, and
`admin-settings` / `database` all keep the credentialed allow-list; the
`connect/*` endpoints keep it too; and with the flag off there is no
wildcard anywhere.
Frontend:
```bash
cd frontend && npx vitest run --root editor src/portal
```
Verified locally: saas 1307 tests / 0 failures, portal 90 files / 578
tests / 0 failures, typecheck clean on the portal, proprietary, saas and
cloud cascades.
End to end, against a preview with account linking on: link an instance,
open Plan and Usage and confirm the wallet loads with no CORS error;
then open Documents and the audit log and confirm they show the
instance's own activity.
The top-level router picks one of six capabilities. On qwen3:8b it was getting that wrong roughly a quarter of the time. Two changes: six worked examples appended to the router prompt, and `temperature: 0` pinned on the router call. `build_model_settings` only ever set `max_tokens`, so the router ran at qwen3's own defaults (temperature 0.6, top_p 0.95) and the same question could route differently on repeat. The pin is on the router only, so generation elsewhere is unchanged. ## What the numbers say An ablation isolates each factor. 82 labelled turns against a live Ollama, two independent runs each: | variant | mean accuracy | |---|---| | ships today | 76.2% | | + temperature 0 | 80.5% | | + `max_tokens` 4096 | 81.7% | | + examples | **89.6%** | The prompt does the work. `max_tokens` was worth about 1.2 points - one case at this sample size - so **it is no longer part of this PR**. Its real justification was avoiding empty responses when thinking exhausts the budget, and no run recorded a single hard failure, so it was not earning its place. Raising it remains reasonable as a separate safety change. ## Why six examples and not more The first version of this PR carried thirteen, mostly contrasting pairs on the question-versus-edit boundary. Measured against a set with **one example per capability**: | prompt | mean accuracy | prompt tokens | |---|---|---| | 13 examples, boundary-heavy | 87.8% | 648 | | 4 examples, boundary only | 78.0% | 458 | | **6 examples, one per capability** | **89.6%** | **517** | Six is both better and smaller. Cutting to four - still boundary-only - threw the entire gain away, which is the tell: the router's dominant failure was never question-versus-edit, it was bailing to `unsupported` (15 of 19 errors in the original run). A block with no `unsupported`, `pdf_review`, `pdf_create` or `user_spec` example cannot fix the errors that actually dominate. Coverage matters more than volume. ## Also worth knowing Thinking stays on. With the same prompt, thinking on scored 91.5% against 89.0% off in the original run, and took destructive misroutes - read-only turns sent to a file-mutating capability - from three to zero. Reproduce with the harness in #7778: `uv run --group engine python evals/routing/runner.py`
What was wrong in scripts/lint/comment-lint-hook.mjs: The win32 fallback hardcoded task.cmd. Scoop ships only task.exe, so cmd.exe returned exit 1. The hook maps exit 1 to FOUND, so a missing executable became "findings" with an empty report. Task's stderr was captured and discarded, so the actual error never surfaced. The fix (commit b285c0d, 17 insertions): Fall back to bare task on win32 too, still shell: true, so PATHEXT finds task.exe / .cmd / .bat. FOUND with an empty report is now treated as "did not run", exit 1 (non-blocking) instead of exit 2. Keep Task's stderr and print it with that message. ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] Every comment I added says something the code does not ([guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/CODE_COMMENTS.md)) - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )