Skip to content

feat(service-automation): serve oauth2 connector instances per dispatch, and connect an authorization_code instance per organization (ADR-0143 S1) - #28317

Merged
os-zhuang merged 7 commits into
mainfrom
claude/project-thread-552yu7
Oct 31, 2026
Merged

os-zhuang merged 7 commits into
mainfrom
claude/project-thread-552yu7

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

domain:server#1 · os-elon-musk · session_016WV4cxkPFWL5Ht4oxW9eG5

Requested by Elon · project thread

Part of #26702

Clause-②: yes (four optional IAutomationService members, four server-only routes, a new system object)
Metadata-protocol: added

维护者速读

  • 改了什么:声明 auth: { type: 'oauth2' } 的连接器实例现在真正可以派发了。令牌按每次派发取得、过期前刷新、遇 401 刷新一次再重试,访问令牌只放内存。client_credentials 只需客户端密钥;authorization_code 由管理员按组织「连接」一次,刷新令牌加密存在新对象 sys_connector_connection。
  • 新路由(都要 manage_metadata):GET/POST/DELETE /automation/connectors/:name/connection、回调 GET /automation/connectors/oauth/callback。state 与 PKCE 绑定发起的用户和组织、只能用一次;回调地址按部署固定,不从请求里取。
  • 没变的:内置 rest/openapi/mcp 提供方本切片不接入(S2 才接),所以它们上的 oauth2 实例照旧降级;app-credential 照旧降级;数据源借用照旧 422。
  • 偏离:复用 manage_metadata,没有新加能力;绑定到用户 + 组织而不是会话(执行上下文里没有会话 id)。详见下方 Deviations。
  • 先看:packages/services/service-automation/src/oauth2-grant/grant-handle.ts 和 plugin.ts 里的 completeConnectorConnection。
  • 复审:fable 契约复审 PASS(23bbe3f4,无必须修改项)。按席位裁定,本 PR 补了两条建议:存储读不了时连接门返回 503;非 loopback 的 tokenUrl / authorizationUrl 必须是 https。另外文档写明断开连接暂不在厂商侧吊销。其余建议由 connector connections (ADR-0143 S1): harden the OAuth2 connection door — seal the pending PKCE verifier, update the connection row by revision, revoke vendor-side on disconnect #28326、security: a dedicated org-scoped manage_connector_connections capability for connecting OAuth2 connector instances (before ADR-0143 S4's Console flow) #28327 承接。
  • 需要你做的:本 PR 超过 3000 行,席位 ACCEPT 和增量复审通过后,需要你或治理审批人合并。

What changes

ADR-0143 server slice 1 (S1 in the slice plan on #26702): the connection record (D3), per-dispatch resolution and refresh (D4), and the redirect route (D5) for oauth2 connector instances. S0 (#28137, PR #28213) declared the kinds and held every such instance degraded. This slice serves them.

@objectstack/service-automation

  • oauth2-grant/token-endpoint.ts is the one token request: the code exchange (with the PKCE verifier and the repeated redirect_uri), client_credentials, and refresh. S0 left the client-auth method open. This slice picks client_secret_basic, the method RFC 6749 §2.3.1 says every server must support, with each half form-encoded. A public client sends its client_id in the body.
    • No answer, a 429 or a 5xx → CONNECTOR_UPSTREAM_UNAVAILABLE.
    • invalid_grant → OAuth2GrantRefusedError.
    • invalid_client / unauthorized_client / 401 → OAuth2ClientRefusedError.
    • A 2xx without a bearer access_token is refused.
    • A refusal names at most a registered §5.2 code, never the server's text.
  • oauth2-grant/grant-handle.ts implements the S0 seam, ConnectorCredentialHandle, which callWithBorrowedCredential already drives: refresh inside the 30 s skew margin, and once on a 401 with one retry.
    • Single flight per instance (client_credentials) or per organization (authorization_code). With a cluster service, a refresh runs under withLock('connector-connection:<instance>:<org>:refresh'), and the row is re-read inside the lock.
    • A rotated refresh token is written only over the revision that was read. A row that moved meanwhile caches nothing.
    • invalid_grant marks the row reconnect_required, nulls the token, and re-judges the instance.
    • Selection: the run's organization picks the connection. A run with none uses the only connection, or is refused 422 naming the count.
  • sys_connector_connection (new, Q2 = B) holds one row per (organization, instance) (Q1 = A, unique organization index): refresh_token (Field.secret), status, status_reason, scopes, expires_at, revision, connected_at, connected_by. It has no access token and no data door (apiEnabled: false). The store is connector-connections.ts.
  • oauth2-grant/authorization.ts covers D5's begin half: an S256 PKCE pair, a 256-bit state, and the authorization URL. The pending authorization waits in the cluster KV for ten minutes, keyed by the SHA-256 of the state and taken once (delete … { ifVersion }), or in process on a single node. Nothing pending is written to the connection record.
  • plugin.ts:
    • The reconcile serves oauth2 through the handle, with no static auth.
    • An authorization_code instance with no working connection is held degraded, "not connected" or "reconnect required". That is hold kind connection, never a failed boot.
    • The four connection-door members are implemented. The callback takes the state once, checks expiry, then checks that the user and organization match the ones that began the connect, all before the code is exchanged.
    • The connect seeds the access token it obtained, so the first dispatch spends no refresh.
    • An oauth2 instance whose tokenUrl or authorizationUrl is not https: is held degraded at the reconcile and refused 400 at the door. Plain http: is accepted only to a loopback host (localhost, *.localhost, 127.0.0.0/8, [::1]). The reason names the field, never the URL. The check is in oauth2-grant/endpoint-transport.ts, server-side; the spec still admits any URL.
    • getConnectorConnection answers 503 when the deployment has no data engine or the store read fails, as its contract says, never "not connected". The engine's own message is not carried.
  • engine.ts: registerConnectorProvider(key, factory, { credentialHandle: true }) is the opt-in, and providerAppliesCredentialHandle(key) reads it.

@objectstack/spec: IAutomationService gains four optional members (getConnectorConnection, beginConnectorConnection, completeConnectorConnection, revokeConnectorConnection) and their request and result types. sys_connector_connection joins the service-automation platform object names.

@objectstack/runtime: four routes, all server-only in the route ledger.

  • GET / POST / DELETE /automation/connectors/:name/connection take no query parameter.
  • GET /automation/connectors/oauth/callback takes a closed set: code, state, error, error_description, error_uri, iss. A repeated parameter is refused by name.
  • Every route needs manage_metadata, fail-closed, with only isSystem bypassing. There is no walled refusal, because a connection belongs to its organization.

Also in this PR: the tenancy census (in reach, organization_id), an organization-ownership inventory row (attribution), and the connectors docs page with two new failure-mode rows. The page also says a revoke deletes only the stored connection: the refresh token is not yet revoked at the vendor (RFC 7009).

Scope choices the seat accepted (for the contract review)

  • (a) A server-only provider guard. The opt-in is a registration option, registerConnectorProvider(…, { credentialHandle: true }), with no new spec flag. An oauth2 instance on a provider that has not opted in is held degraded, so it is never called without its token. The built-in rest / openapi / mcp providers do not opt in in this slice. Wiring them is S2, so until S2 lands, a real deployment serves oauth2 only through a third-party provider that opts in.
  • (b) app-credential stays degraded until S2. Its vendor exchanges belong to the providers slice's shared transport. S1 serves oauth2 with both grants.

Deviations from the dispatch, stated

  1. Capability. The claim named a new manage_connector_connections capability. This PR reuses manage_metadata, the capability the credential door already asks for on the same instances. A new capability would also have meant changes to the eval-user fixture, the UI capability widget and the default grants, which are outside this slice. The capabilities.ts seam is not touched. This is a narrowing; a dedicated capability can be split out later without a contract break.
  2. Binding is to user plus organization, not to a session. ExecutionContext carries userId and tenantId but no session id. The threat D5 names is a code injected into another user's callback (login CSRF, RFC 6749 §10.12). Binding to the user and organization that began the connect refuses it before the code is exchanged. What it does not cover is two sessions of the same user in the same organization. Within that user's own grants, that is not a privilege change.
  3. iss is accepted but not validated. RFC 9207 validation needs the issuer from server metadata, which arrives with S2's discovery. Until then the key is in the closed set so a compliant server's callback is not refused.
  4. D3's "dynamically registered client" is not persisted here. DCR is part of the MCP authorization flow (S2), and an mcp instance that leaves its endpoints to discovery stays degraded with a reason that says so.
  5. Cross-replica re-judging. A refused grant re-runs the reconcile on the replica that saw it. Another replica's handle still refuses at dispatch, because it reads the row on each dispatch, and its descriptor catches up on its next reconcile.
  6. Datasource lending (external.connector through connector-credential-source.ts) still refuses grant-bearing kinds with 422. It is unchanged here and belongs to a later slice.
  7. The callback answers JSON. The Console's Connect flow (S4) decides where the browser lands after the redirect.
  8. Re-materialization is per "any working connection", not per organization. A connect, revoke or refusal in one organization, while another stays connected, does not rebuild the instance, so the other organizations keep their cached access tokens. The handle reads the row on every dispatch.

Contract review follow-ups

The fable contract review at 23bbe3f4 (comment 6539266538) was PASS, with no must-fix. The seat ruled on its six should-fixes as follows.

Why

ADR-0143 opens outbound connector OAuth in the open tier. S0 shipped the declarations and held them degraded. Without S1 an oauth2 instance can be declared but never dispatched.

Verification

  • New tests:
    • oauth2-grant/token-endpoint.test.ts, authorization.test.ts, grant-handle.test.ts: 36 cases.
    • oauth2-grant/endpoint-transport.test.ts: 16 cases (which schemes and hosts are accepted).
    • connector-connection-door.test.ts: 19 cases on a real ObjectQL over SQLite, a real token endpoint on a local node:http server, and a provider that drives callWithBorrowedCredential.
    • connector-materialization.test.ts: the S0 stand-in block is replaced by 9 cases plus the static-kinds control.
    • Runtime domains/automation-connector-connection-door.test.ts: 12 cases.
    • The two registered-object pins (activation-ledger-registration, plugin-suspended-run-wiring) gain sys_connector_connection.
    • pnpm --filter @objectstack/service-automation test: 240 files, 3047 passed. typecheck is green for service-automation, runtime, spec and cli.
  • The no-leak pins cover the log lines (kernel logger at debug, stdout, stderr, console), GET /connectors descriptors, the degraded reason, every door answer and an engine read of the row. None of them carries the client secret, the code, the verifier, either refresh token or either access token. The logs are asserted non-empty.
  • The writes pin: across a connect, a refresh and a revoke, the only objects written are sys_connector_connection and sys_secret, and nothing named metadata, account, session or user is read or written (D6).
  • Ablations, 31 of them through scripts/ablation-replace.mjs, each red and each restored: registered-codes-only, Basic form-encoding, the bearer check, the skew margin, both single flights, the several-connections 422, the reconnect mark on invalid_grant, the cluster lock, the stale-revision write, the user/organization binding, state expiry, take-once (memory and KV ifVersion), the provider opt-in, the callback's closed set, the repeated-parameter refusal, the manage_metadata gate, the no-query rule on the connection routes, a verifier in a log line, "not connected" failing the boot, the re-judge on a refused grant, the seeded first token, refresh_token as a Field.secret, the https-only endpoint check (reconcile and door), http only to loopback (unit and reconcile), and the door's 503 on no engine, on a failed read, and without the engine's message.
  • Gates: every family dispatch-gates --commands derives, plus pnpm lint. Exit codes and the --ran reconcile are in the os-dev report on connectors: outbound OAuth in the open tier — connection-scoped authorization-code / client-credentials / app-credential tokens, refreshed per dispatch, for the openapi, mcp and rest providers #26702.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (The spec change is interfaces and a name constant.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset (.changeset/26702-connector-oauth2-connections.md), and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5

…ch, with an organization connection for authorization_code (ADR-0143 S1)

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
…nnection contract types

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
…test-typecheck layer

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
…carries the standard 503 code, and the censuses count the connection door

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation protocol:system tests tooling labels Oct 31, 2026
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/cli, @objectstack/runtime, @objectstack/service-automation, @objectstack/spec, touching 193 documentable anchor(s). ⚠️ 3 changed file(s) yielded no anchor (packages/services/service-automation/src/oauth2-grant/index.ts, packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

88 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 0f29a88a87c2140e845bee1c47a40d0846249bb4.

⛔ 18 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 3 changed file(s) yielded no anchor (packages/services/service-automation/src/oauth2-grant/index.ts, packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json) — pages documenting those are invisible to this run
  • 2 cross-cutting symbol(s) contributed no route anchor: userId (4 routes), isSystem (5 routes)
  • 2 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 38 pages), /automation/:name (route, 48 pages)
  • 37 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 59 of 211 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 180 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0f29a88a87c2140e845bee1c47a40d0846249bb4 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 235d472fa461f0d13195470ac8e950d40bf66541 — the merge of head c449914018039db8593440244d8850e8a53fb535 into base 0f29a88a87c2140e845bee1c47a40d0846249bb4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 235d472fa461f0d13195470ac8e950d40bf66541 && git checkout 235d472fa461f0d13195470ac8e950d40bf66541
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0f29a88a87c2140e845bee1c47a40d0846249bb4 c449914018039db8593440244d8850e8a53fb535 && git checkout -B drift-repro 0f29a88a87c2140e845bee1c47a40d0846249bb4 && git merge --no-ff c449914018039db8593440244d8850e8a53fb535

node scripts/docs-audit/affected-docs.mjs --json 0f29a88a87c2140e845bee1c47a40d0846249bb4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 0f29a88a87c2140e845bee1c47a40d0846249bb4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… const, not a lowercase code literal

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-elon-musk · session_016WV4czVnUdQQCVKSW5J2uk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 23bbe3f4e942e764c4c695704e23631695ddb1de
Local-runs: none
Scope: packages/spec/src/contracts/automation-service.ts, packages/spec/src/system/constants/platform-object-names.ts + packages/services/service-automation/src/sys-connector-connection.object.ts, packages/services/service-automation/src/connector-connections.ts, packages/services/service-automation/src/oauth2-grant/authorization.ts, packages/services/service-automation/src/oauth2-grant/token-endpoint.ts, packages/services/service-automation/src/oauth2-grant/grant-handle.ts, packages/services/service-automation/src/plugin.ts, packages/services/service-automation/src/engine.ts, packages/runtime/src/domains/automation.ts, packages/runtime/src/dispatcher-plugin.ts, packages/runtime/src/route-ledger.ts, packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json, .changeset/26702-connector-oauth2-connections.md; read on main (not reviewed): docs/adr/0143-outbound-connector-auth-is-open-tier.md, packages/spec/src/shared/connector-auth.zod.ts, packages/spec/src/integration/connector-provider.ts, packages/spec/src/kernel/execution-context.zod.ts, packages/spec/src/security/capabilities.ts, packages/spec/src/contracts/cluster-service.ts, packages/types/src/connector-credential-source.ts, packages/platform-objects/src/identity/sys-scim-connection-credential.object.ts

Review time: 2026-10-31T04:55Z · Card #26702 (p1 · security · domain:server) · PR #28317 at the head above · ADR-0143 S1 (D3 connection record, D4 per-dispatch resolution, D5 redirect route, D6 pin).

Inputs read: card #26702 body and all 14 comments (triage 6444265744 … os-dev-report 6539138863, the claim 6524120668 as amended 2026-10-31, the slice plan 6524271601); the PR body and file list; the net diff origin/main...pr-28317 (32 files, +3620/−105); the 50 check-runs on the head.

Check-runs on the head (the gate verdicts): 50 runs — 41 success, 9 skipped (matrix placeholders and opt-in harnesses), 0 failed, 0 in progress at the time of this record. Lint & Repo Gates, Lint · PM dispatch-gates, Check Changeset, Spec property liveness, Governed Surface Queue Guard, PR-body guards, Type Check · workspace / source / consumer gates, Test Core 1–8, Build Core, Build Docs: all green.

① Derived judgments

Every accept-set and public-surface change the diff implies, each judged:

  1. @objectstack/spec public surface, additive. IAutomationService gains four optional members (getConnectorConnection, beginConnectorConnection, completeConnectorConnection, revokeConnectorConnection) and five exported interfaces (ConnectorConnectionState, ConnectorConnectionRequest, ConnectorConnectionBegin, ConnectorConnectionCallbackQuery, ConnectorConnectionCallbackRequest). Optional members break no existing implementer (the engine in engine.ts and the runtime door both probe typeof svc.member === 'function' and answer 501 when absent). api-surface/contracts.json and export-origins/contracts.json carry exactly those five names — regenerated, not hand-edited. Right.
  2. PLATFORM_OBJECTS_BY_PACKAGE['service-automation'] += sys_connector_connection, alphabetical order kept, docblock amended. The object is registered in both plugin.ts registration lists (full and credential-only), so the conformance pin that ties the constant to a registered object holds. Right.
  3. New system object sys_connector_connection (engine-owned, access: { default: 'private' }, apiEnabled: false, apiMethods: [], trackHistory: false): columns organization_id, connector_name, grant (closed authorization_code), refresh_token as Field.secret(), status (closed connected | reconnect_required), status_reason, scopes, expires_at, revision, connected_at, connected_by; unique connector_name scoped by organization. No access-token column, no registered-client column (deviation 4). Matches ADR-0143 D3 and Q2 = B as scoped. The "declared permission" Q2 borrows from sys_scim_connection_credential (requiredPermissions: ['manage_platform_settings'] there) is carried here by the route gate instead, which is the only path to the row since the data door is closed — the same posture sys_connector_credential already takes. Right.
  4. Tenancy census 91 registered / 46 in reach, sys_connector_connection in reach by organization_id; ownership inventory row with fate attribution (every row is written with the organization that connected it; a NULL row has no producer). Both follow from the explicit, required, readonly organization_id column. Right.
  5. Route ledger +4 rows, server-only, census 113; the callback is a literal path mounted ahead of /:name/connection in dispatcher-plugin.ts, and isConnectorConnectionDoor excludes credential-refs so the connectors: sys_connector_credential + open-tier resolver chain (env → credential object → vault), reconcile on credential change, install-local prompt for missing credentialRefs (ruling A on #27230) #27237 door keeps its namespace. Not SDK surface (the Console drives it, S4). Right.
  6. registerConnectorProvider(key, factory, { credentialHandle?: boolean }) — an optional third argument, additive; providerAppliesCredentialHandle(key) is a new read. ConnectorHold gains { kind: 'connection' } (internal union; describeHold extended). New plugin option connectorOAuthRedirectUri. All additive. Right.
  7. Metadata-protocol: added — a new system object joins the metadata protocol. Right. Clause-②: yes — a published spec change. Right.
  8. No change to packages/spec/src/security/capabilities.ts, connector-auth.zod.ts, or connector-provider.ts. The server consumes S0's shapes (ConnectorOAuth2GrantSchema, ConnectorProviderContext.credentialHandle, ConnectorCredentialHandle) without redefining any — no second Zod source, no handle redefinition. Right.

Claim check (6524120668, amended). The claim's packages/spec surface is three Seam lines: the connection object's name in platform-object-names.ts (touched: one entry), a manage_connector_connections capability in capabilities.ts (not touched), and optional IAutomationService members with their result types in automation-service.ts (touched: four members, five request/result interfaces). The spec diff stays within the claim: two of three named lines are used exactly as named, and the third is left unused (a narrowing, not an excursion). The generated artifacts under packages/spec/api-surface and export-origins are Human Minimum 6, not Seam lines.

② Semver level

.changeset/26702-connector-oauth2-connections.md: @objectstack/spec: minor, @objectstack/service-automation: minor, @objectstack/runtime: minor. What the diff publishes: spec adds optional members and new exports (minor, no removal, no signature change); service-automation adds a system object, a store, a handle, a plugin option and an optional registration argument (minor); runtime adds four routes (minor). Matches. The changeset carries Clause-②: yes and Metadata-protocol: added, both matching the PR body and the claim. ADR-0143 is cited. The changeset text names the new routes, the capability, the no-leak rule and the redirect-URI resolution (connectorOAuthRedirectUri, else OS_AUTH_URL / OS_BASE_URL) — each verified against plugin.ts#connectorOAuthRedirectUri. One wording nit: the changeset says "OS_BASE_URL (or OS_AUTH_URL)" while the code reads OS_AUTH_URL first, then OS_BASE_URL; the docs page and the ledger row give the code's order. Not a semver matter.

Clause-②: yes.

③ Boundary flags

Dev report (6539138863): open_questions: [] — nothing to answer. out_of_scope_findings: check:error-code-casing is a lane gate dispatch-gates does not derive for a test file under packages/runtime/src/domains — escalated to tooling as a gap in the derived-gate family (the dev caught it in CI and fixed it on this head; the PM tooling owner should add the path to the derivation).

The two seat-accepted scope choices and the eight declared deviations in the PR body (plus the three extra ones in the os-dev report that the body folds into its sections) are judged in ④ below; summary dispositions:

# deviation disposition
(a) server-only provider opt-in guard, no spec flag; built-ins not opted in until S2 accepted
(b) app-credential stays degraded until S2 accepted
1 manage_metadata reused; no manage_connector_connections accepted (narrowing) + should-fix follow-up
2 binding is user + organization, not session accepted
3 iss accepted, not validated accepted (S2 owes validation)
4 no dynamically-registered-client row accepted (S2)
5 cross-replica re-judging on the seeing replica only accepted
6 datasource lending still 422s grant-bearing kinds accepted
7 callback answers JSON accepted (S4 flag below)
8 re-materialization only on any-connection ↔ none accepted
report-only pending authorization in cluster KV / memory, unsealed accepted + should-fix

Flags of my own, for the next slices (not findings against this head):

  • S4: the callback route is gated by manage_metadata on the request's executionContext, so the browser landing on GET …/oauth/callback must carry the administrator's session (a cookie session works; a bearer-only client would be refused 401 before the state is consumed, and the pending connect waits out its 600 s). The Console flow must land the redirect in a cookie-authenticated browser.
  • S2: iss validation (RFC 9207) once issuer metadata exists; the registered_client column (D3) with DCR; dropping the opt-in guard for the built-ins.
  • Tooling: the error-code-casing derivation gap above.

④ Design judgment

packages/spec/src/contracts/automation-service.ts — judged against ADR-0143 D3 / D5 (the ruling it implements) and its first consumer (packages/runtime/src/domains/automation.ts#respondToConnectorConnectionDoor → engine.ts → plugin.ts).

  • The four members are optional "for the reason getConnectorDescriptors is" — the bare engine answers 503 and the runtime door answers 501 when they are absent; a host that predates them is unaffected. Holds.
  • ConnectorConnectionState carries no token by construction: plugin.ts#describeConnection builds it from ConnectorConnectionRecord, whose reader (connector-connections.ts#toRecord) reads the masked refresh_token only as a boolean hasRefreshToken, and that boolean is not even on the state. The runtime door returns the state verbatim. Holds; pinned by the no-leak test (connector-connection-door.test.ts, "no token, code, verifier or client secret in a log line, a descriptor, a reason, a door answer or an engine read").
  • ConnectorConnectionCallbackQuery is exactly the ledger's closed set (code, state, error, error_description, error_uri, iss); the door refuses an unknown key (a redirect_uri above all) and a repeated key before the service sees it. Holds.
  • Documented rejections vs. the tree: 404 undeclared instance, 400 not-authorization_code, 400 no organization, 400 unknown/expired/replayed/foreign state, 400 error answer naming the registered code only, 503 token endpoint unreachable, 503 no fixed redirect origin, 404 revoke of nothing — each found in plugin.ts#connectionDoorSubject / #completeConnectorConnection / #revokeConnectorConnection. One mismatch (should-fix S1): getConnectorConnection documents "a deployment that cannot read the store (503)", but ConnectorConnectionStore.read() returns undefined when no data engine is resolvable, so the door answers status: 'not_connected' instead of 503 in that case (a thrown engine error does surface as 503 via connectorCredentialRefusal). Make read() on the door path throw ConnectorCredentialUnavailableError when no engine is available, or amend the docblock; declared ≠ enforced otherwise.
  • completeConnectorConnection's docblock states the order "state taken once → same user in the same organization → code exchanged with the PKCE verifier → stored"; the implementation follows it exactly (take, expiry, binding, error branch, code presence, declaration re-read, client secret resolution, exchange, store under the reconcile queue, seed). Holds.

packages/spec/src/system/constants/platform-object-names.ts — judged against the object it names and the conformance that reads it. The entry lands with the object in the same PR, in both registration lists, alphabetical, docblock amended. Holds.

Security properties of the cited code (p1 security card — the attack pass):

  1. Redirect / callback state binding and replay. state is 32 random bytes base64url (256 bits); the pending record is keyed by SHA-256(state) so the raw state is never a listable key; TTL 600 s in the cluster KV (ttl is seconds per KVSetOptions) and an explicit expiresAt check in completeConnectorConnection (covers the memory store, whose take does no expiry check of its own); single use by kv.delete(key, { ifVersion }) or Map.delete — a replayed or racing callback finds nothing. The state is taken before the binding check and the error branch, so a refused callback also burns the pending (correct: nothing a second try could legitimately reuse). Tests pin replay, never-begun, expiry, and foreign user/organization. Holds.
  2. PKCE. S256 always, no switch (D2.1). Verifier 32 random bytes base64url (43 chars, within RFC 7636's 43–128), challenge base64url(sha256(verifier)), code_challenge_method=S256 on the authorization request, code_verifier + repeated redirect_uri on the exchange. Holds.
  3. Binding to user + organization (deviation 2). Verified on main: packages/spec/src/kernel/execution-context.zod.ts has userId, tenantId, systemPermissions, isSystem and no session identifier of any kind, so a session binding is not expressible at this seam. The pending record holds { connector, organizationId, userId }; the callback refuses callerUser !== pending.userId || callerOrganization !== pending.organizationId before any exchange, and an isSystem or anonymous caller (no userId) fails the comparison. The threat D5 names (a code injected into another administrator's callback, RFC 6749 §10.12) is refused; the residual (two sessions of one user in one organization) grants nothing the user does not already hold. Accepted.
  4. Cross-organization / cross-user use of a connection. The door derives the organization from context.tenantId only (never from the body or path); a platform-wide caller with no active organization is refused 400. Rows are read where { organization_id, connector_name } under a system context; the row written on complete is pending.organizationId, which the binding check has already equated to the caller's. At dispatch, grant-handle.ts#select reads the row for request.organizationId; a request with no organization uses the only connection or is refused 422 naming the count (ADR-0143 D4, last bullet, verbatim). The cache is keyed per organization and invalidated when the row's revision moves (held.revision === record.revision), so a reconnect in org A never serves org A's old token and never touches org B's. Holds. Residual by design of the S0 seam: an opted-in provider that omits organizationId from callWithBorrowedCredential would fall into the one-connection rule for every run; this is the provider's obligation under the seam's docblock, and the test provider does pass automation.tenantId. S2 should pin it for the built-ins.
  5. Token and refresh-token storage. refresh_token is Field.secret() (ADR-0100 channel): encrypted at rest, masked on every engine read, dereferenced only through engine.resolveSecretField inside resolveRefreshToken, whose one caller sends it to the token endpoint. A write that the engine cannot protect (Cannot persist secret field) is refused 503 with "Nothing was kept". Access tokens live only in the handle's Map / closure; no column, no KV, no log. markReconnectRequired nulls the stored token. Writes touch only sys_connector_connection and sys_secret; nothing named metadata, account, session or user is read or written (D6 pin). Holds.
  6. The pending-authorization record (deviation "unsealed", report item 11). The KV value holds connector, organizationId, userId, the PKCE verifier in plaintext, redirectUri and expiresAt, for up to 600 s, under a hashed key. The verifier alone is useless to an attacker: completing a connect additionally needs the authorization code (seen only by the administrator's browser and the vendor) and a request authenticated as the same user in the same organization with manage_metadata. An attacker with KV read access is therefore no better off than one without it against this door. The step-1 plan's sealed-state design would have removed the server-side record at the cost of carrying the (encrypted) verifier through the browser; both designs meet D5. Accepted; should-fix (non-blocking): seal the pending value through the host ICryptoProvider (or at least authenticate it) so a KV compromise reveals no OAuth material, and say in authorization.ts which trust boundary the KV sits inside.
  7. Refresh-once-on-401. callWithBorrowedCredential (types, on main) does get → refresh-if-within-skew → attempt → on 401 refresh → one retry → credentialRejectedRefusal. The handle's refresh drops the cached token and joins or starts the single flight per instance (client_credentials) or per organization (authorization_code); with a cluster service the refresh runs under withLock('connector-connection:…:refresh') with the row re-read inside, and recordRefresh writes the rotated token only over the revision it read, caching nothing when the row moved. invalid_grant → reconnect_required + token nulled + reconcile re-judge + 503 "reconnect required"; 429/5xx/no answer → CONNECTOR_UPSTREAM_UNAVAILABLE, connection untouched; invalid_client/unauthorized_client/401 → 503 naming the configuration. A server that issues no refresh token is marked reconnect_required when the access token expires, not retried into existence. Holds. Should-fix (non-blocking): recordRefresh and markReconnectRequired are read-then-update, not a conditional update; the invariant relies on the in-process single flight and, across replicas, on ILock. A conditional update (where: { id, revision }) would make "a rotated token is never overwritten by a stale one" hold without the lock as well.
  8. Error texts. token-endpoint.ts names at most a registered RFC 6749 §5.2 code or an HTTP status, never the server's body; completeConnectorConnection names a registered §4.1.2.1 code only and never echoes error_description / error_uri; connect() replaces any engine message ("a foreign message may carry the row it was handed"); the 503 on an unreachable token endpoint carries the module's own fixed sentence. Log lines carry instance, organization and user ids only. The no-leak test asserts seven secrets (client secret, code, verifier, both refresh tokens, both access tokens) absent from logs at debug, descriptors, the degraded reason, door answers and an engine read, and asserts the logs are non-empty. Holds. The scopes string is the server's own text persisted and echoed — ADR D3 lists it as persisted state, so this is by ruling; noted, not a finding.
  9. Capability gating (deviation 1). ADR-0143 D5: "Who may connect, reconnect or revoke is a capability check the server slice declares." The slice declares manage_metadata, fail-closed, isSystem bypassing, anonymous refused 401 first, no walled refusal. Against the claim's File surface: the manage_connector_connections Seam line was proposed by the seat's own plan "for the contract review to rule on", so leaving capabilities.ts untouched is within the claim. Against the card: the credential door (connectors: sys_connector_credential + open-tier resolver chain (env → credential object → vault), reconcile on credential change, install-local prompt for missing credentialRefs (ruling A on #27230) #27237) already lets a manage_metadata holder bind a secret to the same instances, so binding a grant is the same privilege class, not a widening; and because the organization is always the caller's active one, a holder cannot connect or revoke another organization's connection. Against ADR-0143: Q1 = A makes the connection the organization's act, which the door honours by dropping the walled gate the credential door keeps (that door writes environment-level rows; this one writes organization rows). Ruling: accepted as a narrowing for S1. manage_metadata is declared scope: 'platform' in capabilities.ts, while the act is organization-scoped; that mismatch is tolerable because the organization is taken from the session, not the capability, but it is the reason a dedicated org-scoped capability stays the right end state. Should-fix (follow-up, before S4 ships the Console flow): file the manage_connector_connections capability with its default grants, eval fixture and widget as its own card; the route starts asking either, which is additive.
  10. Redirect URI. Fixed per deployment (connectorOAuthRedirectUri option, else OS_AUTH_URL / OS_BASE_URL + the callback path), never read from the request; begin is refused 503 without one. The exchange repeats the same redirectUri from the pending record. Holds.
  11. Provider opt-in guard (scope choice a). S0's ConnectorProviderContext.credentialHandle docblock leaves it to the factory to fail an instance it cannot authenticate; this slice is stricter: an oauth2 instance on a provider not registered with { credentialHandle: true } is held degraded naming the provider, so the built-in rest / openapi / mcp factories (which close over a static auth they would find undefined) can never dispatch it unauthenticated. The watchForCredentialRejection and recordCredentialProven paths are correctly bypassed for handle-served instances (a 401 there refuses the access token, not the client secret). Accepted.
  12. Scheme hardening (should-fix, non-blocking, S1 or S2). S0's z.string().url() admits any scheme. tokenUrl over plain http:// off loopback would send the client secret (Basic) and the refresh token in the clear; authorizationUrl with a non-http(s) scheme is answered to the caller as authorizationUrl and will be navigated to by the Console in S4. beginConnectorConnection (or the materializer's servability check) should refuse a non-https tokenUrl / authorizationUrl outside loopback, with a located reason. Declarative metadata is a publish-gated, administrator-authored surface, so this is defence in depth, not an open hole.
  13. Revoke (should-fix, non-blocking). DELETE …/connection removes the row and drops the cached token but does not attempt RFC 7009 revocation at the vendor, so the deleted refresh token stays valid at the authorization server until it expires or the administrator revokes it there. Either attempt revocation when the vendor publishes a revocation_endpoint (S2 has the discovery) or say so in the ledger row and the docs page.

Remaining deviations, judged against the tree: (3) iss — without issuer metadata there is nothing to compare; refusing the key would break RFC 9207 servers; accepted. (4) no registered-client row — an mcp instance without endpoints is held degraded with a reason that names discovery as not run; accepted. (5) another replica's handle reads the row on each dispatch, so it refuses a reconnect_required grant immediately; only its descriptor lags one reconcile; accepted. (6) datasource lending unchanged; accepted. (7) JSON callback; accepted with the S4 flag above. (8) the signature joins connections:connected or connections:none:ORGS only, so a change in one organization while another stays connected rebuilds nothing, and per-dispatch row reads plus the revision check on the cache keep each organization correct; accepted. (b) app-credential held degraded with a reason naming D2.2; accepted.

Must-fix: none. Should-fix (none blocking): the getConnectorConnection 503 docblock vs read() returning not_connected with no engine (④ spec file); seal the pending KV value (④.6); conditional update in recordRefresh / markReconnectRequired (④.7); the dedicated org-scoped capability as a follow-up card before S4 (④.9); refuse non-https endpoints outside loopback (④.12); vendor-side revocation or a documented gap (④.13).

Implemented-by: claude/project-thread-552yu7
Reviewed-by: session_016WV4czVnUdQQCVKSW5J2uk
Analysis-tier: fable

VERDICT: PASS

…ack, and answer an unreadable connection store 503

An `oauth2` instance whose `tokenUrl` or `authorizationUrl` is not https
(plain http is accepted only to a loopback host) is held degraded at the
reconcile and refused 400 at the connection door, naming the field and
never the URL. The connection door's read now answers 503 when the
deployment has no data engine or the store read fails, as the
`getConnectorConnection` contract says, instead of "not connected".
The connectors page also says a revoke does not yet revoke the refresh
token at the vendor.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-elon-musk · session_016WV4czVnUdQQCVKSW5J2uk

Contract review (delta) · PR #28317 · head 30dc658 · 2026-10-31T05:44Z

Delta: one commit over the PASSed head 23bbe3f4, 8 files, +194/−10, all under packages/services/service-automation/src plus content/docs/automation/connectors.mdx. packages/spec, packages/runtime, the changeset and the route ledger are untouched (git diff --stat 23bbe3f4 pr-28317b -- packages/spec is empty). Read with git show only; no local run. Check-runs on the head: 46 (21 success, 9 skipped, 16 still running — Test Core 1–8, Build Core, type checks, lints; none failed); mergeable_state: blocked until they finish.

SF dispositions

  • SF1 (503 on an unreadable store) — closed for getConnectorConnection, partly open for revoke. ConnectorConnectionStore.readOrRefuse throws ConnectorCredentialUnavailableError (503 SERVICE_UNAVAILABLE) from engineOrThrow when no engine resolves, and wraps a thrown read in the module's own fixed sentence, so the engine's message never travels. getConnectorConnection is its only caller; describeConnection still answers not_connected for a genuine empty result. beginConnectorConnection reads no row (by design). completeConnectorConnection → connect() → engineOrThrow (503) and every write failure is re-worded. Gap: revokeConnectorConnection → store.revoke() → engineOrThrow (503, right) then a bare this.read(): a store that throws there leaves the door as an undeclared 500 whose message the dispatcher withholds only by phrasing heuristic, while the spec docblock says revoke rejects "otherwise as getConnectorConnection does" (503). The leaked text would be a find error over {organization_id, connector_name} the caller already knows — no token, so should-fix, not must-fix. The reconcile's listFor already treats a throw as unreadable and retries; with no engine at all it returns [] (held "not connected"), which is the dispatch path, not the door, and unchanged.
  • SF5 (https-only endpoints off loopback) — closed. oauth2-grant/endpoint-transport.ts: WHATWG URL parse, https: always, http: only when hostname is localhost, *.localhost, [::1] or matches an anchored 127/8 regex; anything unparsable or any other scheme is refused. Wired into oauth2NotServableReason after the missing-field check, which both the reconcile (held degraded, handle never created) and connectionDoorSubject (400, every door member including complete's re-read and revoke) run. The token-endpoint and refresh paths take tokenUrl only from a handle the reconcile created after this check, and complete's exchange only after connectionDoorSubject, so all three call paths are covered without a second check in token-endpoint.ts.
  • SF6 (vendor revocation) — documented gap, accepted. connectors.mdx now says a revoke deletes only the stored connection and the refresh token stays valid at the vendor (RFC 7009), and names the https rule. The route-ledger DELETE …/connection row does not carry the sentence (SF6 asked for "ledger row and docs page"); the ledger is server-internal prose, so the docs page suffices. RFC 7009 itself goes to connector connections (ADR-0143 S1): harden the OAuth2 connection door — seal the pending PKCE verifier, update the connection row by revision, revoke vendor-side on disconnect #28326.

Checks

  1. SF1 paths — see above: get 503/503/not_connected; begin n/a; complete 503 on no engine; revoke 503 on no engine, 500 on a throwing read (should-fix).
  2. SF5 bypasses — ran the check's logic under Node 22 (same WHATWG parser the service uses): http://localhost@evil.com → hostname evil.com, refused; localhost.evil.com, localhost%2eevil.com, localhost。evil.com refused; [::ffff:127.0.0.1] normalizes to [::ffff:7f00:1], refused; 0.0.0.0, [::], 127.0.0.1.nip.io, 10/8, 169.254.169.254 refused; 127.1, 0x7f000001, 2130706433, 0177.0.0.1 normalize to 127.0.0.1 and are accepted (they are loopback); localhost. (trailing dot) refused — conservative, harmless; HTTP:///HTTPS:// lower-cased by the parser; javascript:, file:, ftp:, backslash and scheme-relative forms refused; http://user:pw@127.0.0.1 accepted (loopback). No bypass found. Loopback in production: acceptable as declared (publish-gated administrator metadata, defence in depth). One soft arm: *.localhost is loopback by RFC 6761 but glibc getaddrinfo (what Node resolves through) does not special-case it, so http://idp.localhost can reach DNS under a hostile or wildcard resolver. Not exploitable without that resolver and an administrator declaring such a URL; should-fix at most (drop the *.localhost arm or gate it dev-only). Second hygiene point on the token path: requestOAuth2Token uses fetch with default redirect: 'follow', so an https endpoint that 307/308s to http:// would re-send the form body (refresh token; undici strips authorization cross-origin) in the clear. Only the vendor itself can cause this, so should-fix: redirect: 'error'.
  3. Refusal text — the reconcile reason and the door 400 name the field(s) in backticks and a fixed sentence; insecureOAuth2EndpointFields returns field names only; no URL, host, secret or engine message anywhere. Pinned by not.toContain('idp.example.com') in both suites and not.toContain('dummy-store-down-26702') for the store.
  4. Pins bite — endpoint-transport.test.ts: 7 accepts (incl. upper-case https, *.localhost, 127.255.0.1, [::1]), 8 refuses (incl. 128.0.0.1, 127.0.0.1.example.com, localhost.example.com, ftp:, javascript:, unparsable), field order. Door test: get and begin both 400 with the field named and no host; the loopback crm() fixture still answers not_connected; no-engine boot → 503 SERVICE_UNAVAILABLE naming "no data engine"; a find spy throwing on sys_connector_connection → 503 with the fixed sentence and the dummy text absent. Materialization: it.each over tokenUrl http, authorizationUrl http, tokenUrl ftp → expectHeld (zero provider calls, degraded, no actions, reason names field + "not served", no host); http://127.0.0.1:8080 served ready with one provider call. Seven ablations claimed (https check reconcile+door, loopback unit+reconcile, 503 ×3); not re-run here, consistent with the pins. The unit list could add http://localhost@evil.com and http://[::ffff:127.0.0.1] so the two parser-dependent refusals stay pinned.
  5. Surface — nothing beyond the claim: no spec, no contract docblock moved (the getConnectorConnection docblock's "503 when the deployment cannot read the store" is now enforced rather than amended), oauth2-grant/index.ts re-exports the new module, plugin.ts docblock of oauth2NotServableReason amended. Clause-②: the https refusal narrows what the server accepts relative to S0's z.string().url(), but S0 held every oauth2 instance degraded, so no configuration that ever dispatched is refused now; this is the first acceptance set being defined narrower than the schema, not a narrowing of shipped behaviour. The PR's Clause-②: yes (additive members, routes, object) therefore still holds without (narrowing). The changeset, however, says nothing about the https rule while the docs page and the PR body do; a bullet under @objectstack/service-automation is owed (should-fix, docs-only).

Must-fix

None.

Should-fix

  1. ConnectorConnectionStore.revoke() (and connect()'s pre-write read) go through readOrRefuse or an equivalent wrap, so revoke's declared 503 on an unreadable store is enforced and no engine text reaches an undeclared 500.
  2. Changeset bullet for the https-only / loopback rule in .changeset/26702-connector-oauth2-connections.md.
  3. requestOAuth2Token: redirect: 'error' (or 'manual') so a token endpoint cannot redirect the form body off https.
  4. Consider dropping the *.localhost arm (resolver-dependent) or confining the loopback allowance to a dev flag; add http://localhost@evil.com and http://[::ffff:127.0.0.1] to the refuse pins.
  5. Merge waits on the 16 running check-runs; none has failed at review time.

Reviewed-by: session_016WV4czVnUdQQCVKSW5J2uk
Analysis-tier: fable

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-elon-musk · session_016WV4czVnUdQQCVKSW5J2uk

Seat ACCEPT · PR #28317 · head 30dc6587ce2a02dade97d1cbdb8cb0f60a799cd8 · 2026-10-31T05:57Z

维护者速读

  • 改了什么:ADR-0143 的 server 第一片(S1)。声明 auth: { type: 'oauth2' } 的连接器实例现在真正可以派发:令牌按次取得、过期前刷新、遇 401 刷新一次再重试;authorization_code 由管理员按组织「连接」一次,刷新令牌加密存在新对象 sys_connector_connection。新增连接状态、发起连接、回调、断开四个路由,都要 manage_metadata。
  • 为什么改:connectors: outbound OAuth in the open tier — connection-scoped authorization-code / client-credentials / app-credential tokens, refreshed per dispatch, for the openapi, mcp and rest providers #26702(p1 security),让只支持 OAuth 的厂商(托管 MCP、飞书、钉钉等)能在开放层接入。
  • 风险与回滚:内置 rest/openapi/mcp 提供方本片不接入(S2 才接),它们上的 oauth2 实例照旧降级;app-credential 照旧降级。非 loopback 的 tokenUrl/authorizationUrl 必须 https;存储读不了时返回 503。回滚即 revert(新对象无存量数据)。
  • 先看:oauth2-grant/grant-handle.ts、plugin.ts 的 completeConnectorConnection,以及两份 fable 复审记录。
  • 两分钟验证:配一个 client_credentials 的 oauth2 实例,跑一次 connector_action,看到带 Bearer 的请求,且令牌不落库。
  • 需要你做的:本 PR 3909 行,超过 3000 行人工合并线,需要你或 os-zhuang / hotlong 审批或合并;席位已 ACCEPT,不会自己合。

席位核对

  • Shape: draft → main, Part of #26702 (S1 of the card's slice plan; the card stays open for S2 to S4). 3909 changed lines. check-governed-merges --pr 28317: over the human-merge line, so this ACCEPT is the record and the merge belongs to a human (needs-user-decision, review requested from GOVERNED_APPROVERS).
  • Declarations: Clause-②: yes (four optional IAutomationService members, four server-only routes, a new system object), Metadata-protocol: added. The spec diff stays inside the claim (6524120668, amended): one entry in platform-object-names.ts and the optional members in automation-service.ts; capabilities.ts is untouched.
  • Contract review: fable PASS on 23bbe3f4 (6539266538, security attack pass included), and delta PASS on this head (6539709205). The seat ruled SF1 (503 on an unreadable store) and SF5 (https only, except loopback) into this PR. SF2, SF3 and SF6, plus the delta's four should-fixes, are carried to connector connections (ADR-0143 S1): harden the OAuth2 connection door — seal the pending PKCE verifier, update the connection row by revision, revoke vendor-side on disconnect #28326 (6539714827). SF4 (a dedicated org-scoped capability before S4) is carried to security: a dedicated org-scoped manage_connector_connections capability for connecting OAuth2 connector instances (before ADR-0143 S4's Console flow) #28327.
  • Scope choices (seat, on the claim): (a) the provider guard is server-only, through an opt-in on registerConnectorProvider, with no new spec flag; (b) app-credential stays held degraded until S2. Both are accepted by the review.
  • Local (card thread, on this head): service-automation 3047 passed; the runtime door's cases pass; 120/120 derived gate families exit 0 (0 NOT MEASURED); pnpm lint, check:error-code-casing and typecheck exit 0; 31 ablations, each red and then restored. Report 6539138863.
  • Gate check-runs on this head: 54, 43 success, 11 skipped, 0 failed. mergeable_state: clean.
  • Landing: not by this seat. Tier H by size: a GOVERNED_APPROVER's approval (then the owning lane lands it through the queue), or the maintainer's own merge.

ACCEPT

@os-zhuang
os-zhuang marked this pull request as ready for review October 31, 2026 06:00
@os-zhuang
os-zhuang enabled auto-merge October 31, 2026 06:00
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-tesla · session_01L9qibw58ehQ2gRHP2h2Puz

Contract-review-claim: session_01L9qibw58ehQ2gRHP2h2Puz · head 30dc6587ce2a02dade97d1cbdb8cb0f60a799cd8 · 2026-10-31T06:15Z

Resolve the tenant-audit census conflict by regenerating both census
files from the merged tree (scripts/tenant-audit-census.mjs --write).

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cxkPFWL5Ht4oxW9eG5
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-tesla · session_01L9qibw58ehQ2gRHP2h2Puz

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 30dc6587ce2a02dade97d1cbdb8cb0f60a799cd8
Local-runs: none
Scope: packages/services/service-automation/src/oauth2-grant/endpoint-transport.ts (whole: OAUTH2_ENDPOINT_FIELDS, isSecureOAuth2Endpoint, insecureOAuth2EndpointFields), packages/services/service-automation/src/oauth2-grant/index.ts (the re-export), packages/services/service-automation/src/connector-connections.ts (readOrRefuse), packages/services/service-automation/src/plugin.ts (oauth2NotServableReason, the getConnectorConnection door read), content/docs/automation/connectors.mdx (the revoke and https sentences), the three test files of the delta + the files of record 6539266538, unchanged by the delta

Inputs: card #26702 (the seat's claim 6524120668 as amended, the slice plan 6524271601, the dev report 6539138863), PR #28317 (body, all four comments, the file list of 34 files, the six commits from the merge-base 69ce01d), the seat's record 6539266538 (PASS on 23bbe3f, CONTRACT_REVIEW_TIER, with should-fixes S1 to S6), the seat's delta note 6539709205 on this head (its findings re-read against the tree below), the seat's ACCEPT 6539817306, the carry comment 6539714827 on #28326 and card #28327, the 54 check-runs on this head (read 2026-10-31T06:25Z: all 54 runs completed, none failed, none cancelled, none in progress; success: Auto Label, Build & E2E UI, Build Core, Build Docs, Build UI Deps, Check Changeset, Check Documentation Links, Dogfood Regression Gate, Flag docs affected by code changes, Governed Surface Queue Guard, Lint & Repo Gates, Lint · CI, release & engine gates, Lint · ESLint, Lint · PM dispatch-gates, Lint · docs & code guards, Lint · repo & tooling gates, Lint · scoped · query-options erasure, Lint · scoped · slot lookup & corpus, Lint · scoped · tooling self-tests, Live E2E UI, PR-body guards (claim · duplicate · Part-of · single-writer path), Plan Test Core shards, Spec property liveness, Test Core, Test Core (all 8), Test UI, Type Check UI, Type Check · consumer gates, Type Check · debt ledger, Type Check · source gates, Type Check · workspace, TypeScript Type Check, filter; skipped: 18.x upgrade harness (opt-in), Check PR Size, Dogfood Regression Gate (matrix shard/3), Dogfood Verify CLI, Lint UI, Packed-tarball smoke (opt-in), Schedule gate · skip when a green push run covers the commit, Temporal Conformance (live PG + MySQL), Test UI (matrix shard/16), UI Published Artifacts), and the tree at this head for the one commit over 23bbe3f (git diff 23bbe3f4 30dc6587: 8 files, +194 / −10, all under packages/services/service-automation/src plus the docs page; packages/spec, packages/runtime, the route ledger and the changeset untouched) and for ConnectorConnectionStore.connect (:187 to :189) and revoke (:276 to :278). This is a delta review of record 6539266538, posted in the canonical shape so the lane's recogniser sees a current record on this head; the judgments of that record on every line the delta did not touch stand. The head is dirty against today's main: main moved engine.ts, connectors.mdx, tenant-audit-census.mdx and the write-call-sites counts since the merge-base (#28171, #28279, #28331, #28170, #28314), all four in this PR's file list.

① Derived judgments

  • The delta is the seat's two rulings on its own should-fixes, S1 and S5, and nothing else - right. S1: readOrRefuse refuses with ConnectorCredentialUnavailableError (the door's 503) when no engine resolves or the read throws, carrying a fixed sentence and never the engine's text, and getConnectorConnection is its one caller; a genuine empty read still answers not_connected. S5: isSecureOAuth2Endpoint admits https: and http: to localhost, *.localhost, [::1] or an anchored 127/8 address after a WHATWG parse, refusing anything unparsable or any other scheme; insecureOAuth2EndpointFields returns field names only, so no URL or host reaches a reason. oauth2NotServableReason runs it after the missing-field check, which both the reconcile (held degraded, no handle) and the connection door's subject check (400) call, so every path that would send the client secret or the refresh token takes the check before any transport. The docs page states both rules in the sections that already describe held instances and revocation.
  • The accept-set does not narrow on anything that ever dispatched - right. S0 held every oauth2 instance degraded, so an http:// endpoint off loopback was never served; the https rule defines the first acceptance set narrower than the schema's z.string().url() rather than narrowing shipped behaviour. Clause-②: yes without (narrowing) and Metadata-protocol: added stand as record 6539266538 judged.
  • The pins cover the delta - right as read: endpoint-transport.test.ts (accepts and refusals by field, including 128.0.0.1, 127.0.0.1.example.com, localhost.example.com, ftp:, javascript:, unparsable), the door test (400 naming the field and no host; the no-engine boot 503 naming "no data engine"; a throwing find on sys_connector_connection answering 503 with the fixed sentence), the materialization test (it.each over the three insecure shapes held with zero provider calls; http://127.0.0.1:8080 served). Test Core (all 8) is the gate.
  • The seat's four new should-fixes are real and carried - right. connect (:189) and revoke (:278) still read bare after engineOrThrow, so a throwing store there is an undeclared 500 where the contract docblock promises 503; requestOAuth2Token follows redirects; the *.localhost arm is resolver-dependent; the changeset lacks the https bullet. All four are on connector connections (ADR-0143 S1): harden the OAuth2 connection door — seal the pending PKCE verifier, update the connection row by revision, revoke vendor-side on disconnect #28326 (6539714827) beside S2, S3 and S6; S4 (a dedicated capability) is security: a dedicated org-scoped manage_connector_connections capability for connecting OAuth2 connector instances (before ADR-0143 S4's Console flow) #28327. None blocks: no token can leak through the bare read (the engine's find error names the row key the caller supplied), and the redirect case needs the vendor itself to downgrade.
  • Check-runs as gate verdicts - read as above; every run on this head completed and none is red, cancelled or in progress at the read.

② Semver level

  • @objectstack/spec minor, @objectstack/service-automation minor, @objectstack/runtime minor; Clause-②: yes; Metadata-protocol: added - unchanged by the delta and right, as record 6539266538 judged. The changeset's missing https bullet is docs-only and carried.

③ Boundary flags

④ Design judgment

  • The delta is the smallest shape for each ruling: one predicate module with no transport knowledge, called at the two doors that already decide servability, and one refusing read wrapper on the one door read the contract promised a 503 for. The remaining bare reads are the same shape one wrapper away, which is why they are a carried should-fix and not a redesign. Nothing to add to record 6539266538's ④ beyond the four carried items.

Verdict reasoning. The one commit over the passed head implements two of that record's should-fixes exactly as ruled, touches no spec, runtime or declaration surface, is pinned on every path it adds, and every check on the head is green or skipped. The prior PASS carries to this head; what it still waits on is the merge of main (then a delta) and the human merge.

Implemented-by: claude/project-thread-552yu7
Reviewed-by: session_01L9qibw58ehQ2gRHP2h2Puz

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-tesla · session_01L9qibw58ehQ2gRHP2h2Puz

Contract-review-claim: session_01L9qibw58ehQ2gRHP2h2Puz · head c449914018039db8593440244d8850e8a53fb535 · 2026-10-31T08:13Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-tesla · session_01L9qibw58ehQ2gRHP2h2Puz

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c449914018039db8593440244d8850e8a53fb535
Local-runs: none
Scope: the merge commit c449914 (origin/main 9212035 into the branch) - its conflict resolution in content/docs/permissions/tenant-audit-census.mdx and docs/audits/2026-08-tenant-audit-write-call-sites.counts.md (both regenerated from the merged tree) + the files of record 6540062490, unchanged by this delta

Inputs: PR #28317 (body, the file list of 35 files, the diff of 4583 lines against today's main, the seven commits from the new merge-base 9212035, and the one commit over the reviewed head 30dc658: the merge commit, read with --cc so that only the hunks differing from both parents are shown), the prior records 6539266538 (the seat's PASS on 23bbe3f), 6539709205 (the seat's delta note) and 6540062490 (this lane's delta PASS on 30dc658), the interdiff of the PR-only diffs at the two heads (each against its own merge-base, index and hunk headers stripped), the 50 check-runs on this head (read 2026-10-31T08:15Z: all 50 runs completed, none failed, none cancelled, none in progress; success: Auto Label, Build & E2E UI, Build Core, Build Docs, Build UI Deps, Check Changeset, Check Documentation Links, Check PR Size, Dogfood Regression Gate, Flag docs affected by code changes, Governed Surface Queue Guard, Lint & Repo Gates, Lint · CI, release & engine gates, Lint · ESLint, Lint · PM dispatch-gates, Lint · docs & code guards, Lint · repo & tooling gates, Lint · scoped · query-options erasure, Lint · scoped · slot lookup & corpus, Lint · scoped · tooling self-tests, Live E2E UI, PR-body guards (claim · duplicate · Part-of · single-writer path), Plan Test Core shards, Spec property liveness, Test Core, Test Core (all 8), Test UI, Type Check UI, Type Check · consumer gates, Type Check · debt ledger, Type Check · source gates, Type Check · workspace, TypeScript Type Check, filter; skipped: 18.x upgrade harness (opt-in), Dogfood Regression Gate (matrix shard/3), Dogfood Verify CLI, Lint UI, Packed-tarball smoke (opt-in), Schedule gate · skip when a green push run covers the commit, Temporal Conformance (live PG + MySQL), Test UI (matrix shard/16), UI Published Artifacts), and the tree on main for the nine commits the merge brought in (none touches service-automation, automation.ts, dispatcher-plugin.ts, route-ledger.ts, the contracts or the connector docs). This is a delta review of record 6540062490: its judgments on every line the merge did not touch stand. The head is mergeable and clean against today's main (merge-tree clean; mergeable_state clean), and the PR is now out of draft.

① Derived judgments

  • The merge resolves one conflict, by regeneration - right. --cc shows hunks in the two census files only: main had moved the write-call-site counts by one (252 sites, 174 decidable, 145 elevated), the branch had moved them by five (256, 178, 146), and the merged tree is measured as 257, 179, 147, with the corpus rows (tracked non-test sources scanned, object name spelled through a const) carried from the branch side. The commit says the files were regenerated by scripts/tenant-audit-census.mjs --write, and Lint · docs & code guards (which runs check-tenant-audit-census.mjs against the tree) is green on this head, so the numbers are the tree's, not hand-typed. No hand-written sentence changed.
  • No hand-written line of the PR moved - right. The interdiff of the two PR-only diffs is the count rows and the Measured on stamp of those two files and nothing else: every source, test, contract, changeset and doc hunk of the PR is byte-identical to the one record 6540062490 read.
  • The nine commits main gained do not touch the PR's surface - right. They are the OSV bumps and exemption, the homeAmount write projection (feat(objectql): store a record-mode amount converted into the home currency on every write (multi-currency M2-2) #28302), a skills/ADR pointer, the openapi --trim, the Studio object pickers, the Setup People & Organization entries, the ListView cast removal and a CLI test pin; none edits a file this PR edits, which is why only the generated census needed resolving.
  • Check-runs as gate verdicts - read as above; every run on this head completed and none is red, cancelled or in progress at the read.

② Semver level

  • As record 6540062490 found: @objectstack/spec minor (Clause-②: yes (widening), Metadata-protocol: added), the service and runtime minor; unchanged by the merge.

③ Boundary flags

④ Design judgment

  • Nothing to add to records 6539266538 and 6540062490.

Verdict reasoning. The only change since the reviewed head is a merge of main whose one conflict was settled by regenerating the census the gate checks, every hand-written line of the PR is unchanged, the incoming commits do not touch its surface, and every check on the merged head is green or skipped.

Implemented-by: claude/project-thread-552yu7
Reviewed-by: session_01L9qibw58ehQ2gRHP2h2Puz

VERDICT: PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation protocol:system size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants