Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/28326-oauth2-connection-door-hardening.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@objectstack/service-automation': patch
'@objectstack/runtime': patch
---

fix(service-automation): harden the OAuth2 connector connection door (ADR-0143 S1 follow-up, #28326)

Clause-②: no
Metadata-protocol: none

- **`@objectstack/service-automation`**:
- **The connect that is waiting for its callback is held sealed.** The pending authorization (the PKCE verifier, the user, the organization and the redirect URI) is held in the cluster KV, or in process on a single node, as an AES-256-GCM envelope under a key derived from the raw `state`. The server keeps only the state's digest. A party that can read the KV learns none of those values. A value that is edited, or moved to another state's slot, opens as nothing, and its code is never exchanged. A connect begun before the upgrade and completed after it is refused as expired; begin it again.
- **The connection row is written only over the revision that was read.** A refresh, the "reconnect required" mark and a reconnect are each one conditional write (`where: { id, revision }`), so a stale writer on any replica never overwrites a newer grant. A reconnect that loses such a race is refused `503` and keeps nothing; begin it again.
- **A connection store that cannot be read refuses a connect or a revoke with `503`**, the declared answer, instead of an undeclared `500`.
- **An `oauth2` token endpoint's redirect is refused, never followed.** The token request carries the client secret and the refresh token, and a `307` or `308` would replay them to the `Location` it names.
- **Plain `http://` is accepted only to a loopback host:** `localhost`, a `127.0.0.0/8` address or `[::1]`. A `*.localhost` name is no longer accepted, because the system resolver sends it to DNS like any other name. Every other `tokenUrl` or `authorizationUrl` must be `https://`, or the instance is held degraded with the field named.
- **`@objectstack/runtime`**: the route ledger row for `DELETE /api/v1/automation/connectors/:name/connection` now says that a revoke is local only. No RFC 7009 revocation is sent to the vendor yet, because no revocation endpoint is known for the instance until ADR-0143 S2 discovery. The refresh token stays valid at the vendor until it expires or is revoked there.
29 changes: 23 additions & 6 deletions content/docs/automation/connectors.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,15 @@ a rotating refresh token is never spent twice.
(`not_connected`, `connected`, `reconnect_required`), never a token. The
`state` and the PKCE verifier are bound to the user and organization that
began the connect and are used once; a callback from anyone else is refused
before its code is exchanged. The redirect URI is the deployment's
before its code is exchanged. While the connect waits (ten minutes at most)
it is held in the cluster KV, or in process on a single node, **sealed**
under a key derived from the `state` itself, which the server never stores:
the KV holds only the state's digest and an AES-256-GCM envelope, so a party
that can read the KV learns no verifier, user or organization, and a value
edited or moved there opens as nothing and its code is never exchanged.
Write access to the cluster KV stays inside the platform's trust boundary,
as it is for the cluster lock: such a writer can still delete a waiting
connect. The redirect URI is the deployment's
(`OS_BASE_URL` or `OS_AUTH_URL` plus `/api/v1/automation/connectors/oauth/callback`,
or the automation plugin's `connectorOAuthRedirectUri`), never the
request's; register that URI with the vendor. The refresh token is kept
Expand All @@ -505,18 +513,27 @@ a rotating refresh token is never spent twice.
instance's only connection, and is refused when there are several. When the
vendor refuses the grant (`invalid_grant`), the connection is marked
**reconnect required** and the instance degraded until an administrator
connects it again. A revoke deletes the stored connection only: the server
does not yet revoke the refresh token at the vendor (RFC 7009), so it stays
valid there until it expires or is revoked in the vendor's console.
connects it again. A refresh, and the mark "reconnect required", are each
written only over the revision they read, in one conditional write, so a
stale one never overwrites a newer connect on any replica. A revoke deletes
the stored connection only: the server does not revoke the refresh token at
the vendor (RFC 7009), because no revocation endpoint is known for the
instance yet — the auth declares none, and the discovery that would supply
one is ADR-0143 S2. The refresh token stays valid at the vendor until it
expires or is revoked in the vendor's console; revoke it there too when the
disconnect is a response to a leak.

What is not served yet is held **degraded** with a reason that names it, and
never called without its credential: `app-credential` (its vendor exchanges
are not run yet), an `oauth2` instance on a provider that does not yet ask the
credential handle (the built-in `rest`, `openapi` and `mcp` providers do not
yet), an `mcp` instance that left its endpoints to discovery, and an `oauth2`
instance whose `tokenUrl` or `authorizationUrl` is not `https://` (plain
`http://` is accepted only to a loopback host, so the client secret and the
refresh token never cross a network in the clear). A datasource
`http://` is accepted only to `localhost`, a `127.0.0.0/8` address or
`[::1]` — not a `*.localhost` name, which the system resolver sends to DNS —
so the client secret and the refresh token never cross a network in the
clear; a token endpoint that answers with a redirect is refused rather than
followed). A datasource
that borrows from a grant-bearing instance (`external.connector`) is refused
with `422`. Until then, declare a static kind (`bearer`, `api-key` or `basic`)
to dispatch now; don't emulate a grant by putting a short-lived access token
Expand Down
40 changes: 20 additions & 20 deletions content/docs/permissions/tenant-audit-census.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ what moved this page's population from 225 to 227; nothing about the two sites
changed, only whether this instrument could see them.

**The expensive failure direction is a keyword.** Sites whose receiver the author
typed `any` have no type to read, and there are 63 of them — just over a fifth
typed `any` have no type to read, and there are 62 of them — just over a fifth
of the population, concentrated in exactly the seed and bootstrap paths this
control exists for. Scoring an unreadable receiver as "not an engine" would have
dropped every one of them silently, with a clean exit and a smaller number that
Expand Down Expand Up @@ -132,7 +132,7 @@ are reported as `undecidable` rather than assumed either way.

The same holds twice over for the context. An options argument spelled as a
literal can be read; one spelled `options`, `{ ...opts }`, or handed through a
forwarding shim cannot, and **51 of the 257 sites are spelled that way**. A
forwarding shim cannot, and **51 of the 255 sites are spelled that way**. A
context resolved from an inline literal or a local `const` can be tested for
`isSystem`; one arriving from a helper call cannot.

Expand Down Expand Up @@ -197,10 +197,10 @@ reproduce them. Where it disagrees, it disagrees on the page:

| carried figure | where it survives | this census |
| :--- | :--- | ---: |
| 175 write call sites | quoted in the merged changeset | **257** |
| 175 write call sites | quoted in the merged changeset | **255** |
| 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **29** more whose options argument is unreadable |
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **179 of 257** decidable, **78** undecidable |
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 147 decidably elevated, 0 decidably not, 102 undecidable |
| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **177 of 255** decidable, **78** undecidable |
| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 145 decidably elevated, 0 decidably not, 102 undecidable |
| 141 and 132, two independent re-derivations | the card that filed this work | — |

**The differences are not reconciled, and deliberately so.** The old census's
Expand All @@ -210,18 +210,18 @@ be stated is what this instrument counts, which is written above and re-runnable
at any commit.

Two structural facts do plausibly widen this reading against any hand or regex
one, and both are counted in the generated tables below: the 63 sites reached
through an erased (`any`) receiver, and the 63 that name their object through a
one, and both are counted in the generated tables below: the 62 sites reached
through an erased (`any`) receiver, and the 61 that name their object through a
`const` rather than inline. An instrument that read either the way a person does
would report a smaller number and would not say so.

The fourth row is the one worth flagging to anyone citing it. **The 135 / 77%
figure has no surviving corroboration anywhere in the tree.** This census reads
147 of 257 (57%) as decidably elevated, with 102 more whose elevation is a
145 of 255 (57%) as decidably elevated, with 102 more whose elevation is a
run-time fact — so the claim is neither confirmed nor refuted, and the honest
answer is that a static reading cannot settle it.

⇒ **Cite `2 / 257`, and say what it is**: the sites whose options argument was
⇒ **Cite `2 / 255`, and say what it is**: the sites whose options argument was
READ and holds no tenant context, against a decidably tenancy-enabled object.
That is the control's provable yield surface. ⛔ Do not cite it as "the sites
without tenant context" — **29 further sites** have an options argument this
Expand All @@ -233,29 +233,29 @@ cannot read, and they are neither in nor out.

| what | count |
| :--- | ---: |
| write call sites on the application surface | **257** |
| …whose object name is statically decidable | 179 |
| write call sites on the application surface | **255** |
| …whose object name is statically decidable | 177 |
| …whose object name is chosen at run time | 78 |
| …against an object with tenancy ENABLED | 178 |
| …against an object with tenancy ENABLED | 176 |
| …against an object that declares tenancy off | 1 |
| threading a tenant context | 198 |
| threading a tenant context | 196 |
| PROVABLY carrying none (options read, no context key) | **8** |
| …of those, against a decidably tenancy-enabled object | **2** |
| options argument UNREADABLE — may or may not carry one | 51 |
| …of those, against a decidably tenancy-enabled object | 29 |
| threading a decidably ELEVATED (`isSystem`) context | 147 |
| threading a decidably ELEVATED (`isSystem`) context | 145 |
| threading a context that is decidably NOT elevated | 0 |
| threading a context whose elevation is a run-time fact | 102 |

| how the instrument reached the site | count |
| :--- | ---: |
| receiver carried a readable engine type | 194 |
| receiver erased, placed by the object NAME | 53 |
| receiver carried a readable engine type | 193 |
| receiver erased, placed by the object NAME | 52 |
| receiver erased, placed by an `object: string` PARAMETER | 5 |
| receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 |

| object name spelled inline | 116 |
| object name spelled through a `const` | 63 |
| object name spelled through a `const` | 61 |
| object name is an `object: string` parameter | 7 |
| object name is some other run-time expression | 71 |

Expand Down Expand Up @@ -307,13 +307,13 @@ holds still. They are required to be HERE and to say WHEN they were true;
their values are not compared. The reasoning, and the measurement behind it,
are in `scripts/check-tenant-audit-census.mjs`.

Measured on 2026-10-31 at `30dc6587c`.
Measured on 2026-10-31 at `d06a02507`.

| corpus scale (not enforced) | count |
| :--- | ---: |
| tracked non-test sources scanned | 829 |
| tracked non-test sources scanned | 830 |
| engine-shaped types recognised | 96 |
| declared objects in the registry | 124 |
| same-named calls subtracted as non-engine | 212 |
| same-named calls subtracted as non-engine | 214 |

{/* END GENERATED: tenant-audit-census */}
17 changes: 8 additions & 9 deletions docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution.

| Measure | Value |
|---|---:|
| Write call sites | 257 |
| Object name statically decidable | 179 |
| Write call sites | 255 |
| Object name statically decidable | 177 |
| Object name chosen at run time | 78 |
| Against a tenancy-enabled object | 178 |
| Against a tenancy-enabled object | 176 |
| Against an object declaring tenancy off | 1 |
| Threading a tenant context | 198 |
| Threading a tenant context | 196 |
| Provably carrying none | 8 |
| …and decidably tenancy-enabled | 2 |
| Options argument unreadable | 51 |
| …and decidably tenancy-enabled | 29 |
| Threading a decidably elevated context | 147 |
| Threading a decidably elevated context | 145 |
| Threading a decidably non-elevated context | 0 |
| Threading a context of undecidable elevation | 102 |

Expand Down Expand Up @@ -90,14 +90,14 @@ holds still. They are required to be HERE and to say WHEN they were true;
their values are not compared. The reasoning, and the measurement behind it,
are in `scripts/check-tenant-audit-census.mjs`.

Measured on 2026-10-31 at `30dc6587c`.
Measured on 2026-10-31 at `d06a02507`.

| corpus scale (not enforced) | count |
| :--- | ---: |
| tracked non-test sources scanned | 829 |
| tracked non-test sources scanned | 830 |
| engine-shaped types recognised | 96 |
| declared objects in the registry | 124 |
| same-named calls subtracted as non-engine | 212 |
| same-named calls subtracted as non-engine | 214 |

## Every site

Expand Down Expand Up @@ -205,7 +205,6 @@ Measured on 2026-10-31 at `30dc6587c`.
| `packages/services/service-automation/src/connector-connections.ts` | `delete` | `sys_connector_connection` | enabled | context, elevation undecidable | 1 |
| `packages/services/service-automation/src/connector-connections.ts` | `insert` | `sys_connector_connection` | enabled | context, elevation undecidable | 1 |
| `packages/services/service-automation/src/connector-connections.ts` | `update` | `sys_connector_connection` | enabled | context, elevation undecidable | 1 |
| `packages/services/service-automation/src/connector-connections.ts` | `update` | `sys_connector_connection` | enabled | elevated | 2 |
| `packages/services/service-automation/src/connector-credentials.ts` | `delete` | `sys_connector_credential` | enabled | context, elevation undecidable | 1 |
| `packages/services/service-automation/src/connector-credentials.ts` | `insert` | `sys_connector_credential` | enabled | context, elevation undecidable | 1 |
| `packages/services/service-automation/src/connector-credentials.ts` | `update` | `sys_connector_credential` | enabled | context, elevation undecidable | 1 |
Expand Down
4 changes: 2 additions & 2 deletions packages/runtime/src/route-ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,9 +548,9 @@ export const ROUTE_LEDGER: readonly RouteLedgerEntry[] = [
{ route: 'GET /automation/connectors/:name/connection', domain: '/automation', disposition: 'server-only',
note: "The caller's organization's connection of one declared `oauth2` `authorization_code` instance: `status` (`not_connected` / `connected` / `reconnect_required`), its reason, the granted scopes, the access token's expiry and who connected it. ⛔ Never a token. Refused: an undeclared instance 404 `RESOURCE_NOT_FOUND`; one whose auth is not that grant, or a caller with no organization, 400 `VALIDATION_ERROR`. ⚑ `manage_metadata`, fail-closed, only `isSystem` bypassing; no walled refusal, since a connection belongs to its organization. Closed query set: none. NOT JS-SDK surface on this leg: the driver is the Console's Connect / Reconnect flow (ADR-0143 S4). Pinned in `domains/automation-connector-connection-door.test.ts`" },
{ route: 'POST /automation/connectors/:name/connection', domain: '/automation', disposition: 'server-only',
note: "Begin connecting (or reconnecting) the instance for the caller's organization: answers `{ authorizationUrl, expiresAt }`, the vendor URL carrying a fresh `state` and an `S256` PKCE challenge. Nothing is written to the connection record until the callback completes; the pending authorization (verifier included) waits ten minutes in the cluster KV, or in process on a single node. The redirect URI is the deployment's (`OS_AUTH_URL` / `OS_BASE_URL`, or the plugin option), never the request's — 503 `SERVICE_UNAVAILABLE` when there is none. No body. Same refusals, gates and SDK posture as the GET row above. Pinned in `domains/automation-connector-connection-door.test.ts`" },
note: "Begin connecting (or reconnecting) the instance for the caller's organization: answers `{ authorizationUrl, expiresAt }`, the vendor URL carrying a fresh `state` and an `S256` PKCE challenge. Nothing is written to the connection record until the callback completes; the pending authorization (verifier included) waits ten minutes in the cluster KV, or in process on a single node, sealed (AES-256-GCM) under a key derived from the raw `state`, which the server keeps only as a digest. The redirect URI is the deployment's (`OS_AUTH_URL` / `OS_BASE_URL`, or the plugin option), never the request's — 503 `SERVICE_UNAVAILABLE` when there is none. No body. Same refusals, gates and SDK posture as the GET row above. Pinned in `domains/automation-connector-connection-door.test.ts`" },
{ route: 'DELETE /automation/connectors/:name/connection', domain: '/automation', disposition: 'server-only',
note: "Revoke the caller's organization's connection: the row is removed, the cached access token dropped, and the instance re-judged — its next dispatch there is refused 'not connected'. 404 `RESOURCE_NOT_FOUND` when it was not connected. Same gates and SDK posture as the GET row above. Pinned in `domains/automation-connector-connection-door.test.ts`" },
note: "Revoke the caller's organization's connection: the row is removed, the cached access token dropped, and the instance re-judged — its next dispatch there is refused 'not connected'. ⚑ Local only: no RFC 7009 revocation is sent to the vendor, because no revocation endpoint is known for the instance (the auth declares none; ADR-0143 S2 discovery would supply one), so the refresh token stays valid at the vendor until it expires or is revoked there. 404 `RESOURCE_NOT_FOUND` when it was not connected; 503 `SERVICE_UNAVAILABLE` when the connection store cannot be read. Same gates and SDK posture as the GET row above. Pinned in `domains/automation-connector-connection-door.test.ts`" },
{ route: 'GET /automation/connectors/oauth/callback', domain: '/automation', disposition: 'server-only',
note: "The authorization server's redirect (ADR-0143 D5). Closed query set: `code`, `state`, `error`, `error_description`, `error_uri`, `iss`; an unknown or repeated one is 400 `VALIDATION_ERROR`. The `state` is taken once and must have been begun by the same user in the same organization, else 400 before any code is exchanged; an `error` answer is 400 naming its registered code only. The code is exchanged with the PKCE verifier and the connection stored; answers the connection's status. 503 when the token endpoint cannot be reached. Same capability gate as the rows above: the browser landing here is the administrator's. Pinned in `domains/automation-connector-connection-door.test.ts`" },
{ route: 'GET /automation/_status', domain: '/automation', disposition: 'sdk', client: 'automation.getRuntimeStatus' },
Expand Down
Loading
Loading