Repository navigation
Add secure external API triggers for workflows - #2138
Conversation
…er-owned idempotency
ExternalTriggersController resolved IExternalTriggerIngress directly, so the anonymous delivery path reached the application without passing any MediatR pipeline behavior — the same bypass the run-ledger SSE stream made. Dispatch a marker-less ReceiveExternalTriggerCommand instead, mirroring the anonymous webhook ingress. The command declares no authorization marker, so no authorization behavior selects it, exactly like ReceiveWebhookCommand; the endpoint stays anonymous at [AllowAnonymous]. The outcome type lives in CodeSpace.Messages so the Messages assembly keeps no reference to CodeSpace.Core: ExternalTriggerDeliveryOutcome mirrors the ingress's own outcome, and the handler maps between the two. The HTTP contract is unchanged — the bounded body and its 413, the Bearer -> X-Api-Key -> ?key= precedence, the whitespace-only Idempotency-Key normalization, and the 404/401/400/200/202 outcome mapping all behave as before. ReceiveExternalTriggerCommand joins AnonymousByDesign in the authorization inventory, the same list ReceiveWebhookCommand already sits in.
DeleteAsync soft-deleted the workflow and its activations but left workflow_external_trigger live. Reveal, rotate and rename resolve a slug through that table alone, so after a delete any team member who knew the slug could still read the plaintext bearer key, and the slug stayed reserved for a workflow nobody could list. Soft-delete rather than shred: it matches how ExternalTriggerDefinitionSync already retires a trigger whose node left the definition (ciphertext retained, partial unique index uq_wet_slug freed because it is filtered on deleted_date IS NULL), it is the convention every other retired row in this repo follows, and it preserves the audit history of which key belonged to which endpoint. The row is now marked deleted in the same SaveChangesAsync as the workflow and its activations, so the delete stays atomic.
The confirm panel interpolated the derived slug unconditionally, so opening the rename confirmation and then clearing or mangling the draft left the user reading "Rename the live endpoint to ?" with no reason and no way forward. Reuse the guidance the slug field already shows for that case.
There was a problem hiding this comment.
感謝這個 PR 的細緻工作。key 的處理、tenancy、資料庫層的冪等保證和測試深度都很扎實;重用 IRunStarter / RunSourceEnvelope / ActivationAuthoritySnapshot 也是正確的選擇。
這次標成 Request changes,原因有兩個:
- 有幾項屬於對外公開的契約。一旦有外部呼叫方接入,就再也收不回來。
- 匿名入口有幾處防線比現有的 webhook 和 callback 路徑弱。
合併前:對外契約(現在改成本低,之後改不了)
?key=查詢參數形式的憑證- 位置:
ExternalTriggersController.cs:32,40;編輯器範例在external-trigger-inspector.tsx:47。 - 問題:#2113 的日誌遮罩只涵蓋名為
token的路徑參數,?key=不在範圍內。任何一行帶?key=的 access log,都等於一個長期有效的觸發能力。 - 修法:只接受
Authorization: Bearer與X-Api-Key,curl 範例也一併改掉。
- 位置:
- endpoint 身分
- 問題:目前是人工命名、全域唯一、可改名的 slug,加上單一 key 欄位。實際後果:
- 跨 team 建立同名 slug 會回 409,形成存在性 oracle;
- 401 與 404 的差異讓 endpoint 可被列舉;
- 改名會讓既有呼叫方立即失效;
- rotate 沒有重疊期。
- 修法:URL 改用不可猜的 opaque public id,並在路由加上長度限制;名稱降為可自由修改的 label。憑證改成 current/previous 兩個 slot,並帶到期時間。
- 問題:目前是人工命名、全域唯一、可改名的 slug,加上單一 key 欄位。實際後果:
- 拒絕語意
- 問題:authority 被拒與 completion admission 被拒都回
202 {runId:null}。稽核的 DedupKey 是rejected:{slug}:{idempotencyKey}:不帶 Idempotency-Key 時只留一筆就失明,帶了則每次投遞都新增一筆、無上限。 - 修法:對已通過驗證的呼叫方回傳穩定的非 2xx 錯誤碼。稽核列改用 (endpoint, reason, day) 為鍵,與
PullRequestTriggerAdmission的做法一致。
- 問題:authority 被拒與 completion admission 被拒都回
- 冪等範圍
- 問題:範圍綁在
activation.Id,而ReplaceActivationsAsync每次存檔都會重建 activation。所以「投遞、存檔、再投遞同一個 key」會產生第二個 run。 - 修法:改以 trigger 列 id 為範圍,並存 body hash;同一個 key 搭配不同 body 時回 422。
- 問題:範圍綁在
- 路由遮蔽
- 問題:新增的
GET /api/workflows/external-triggers會遮蔽 slug 剛好為external-triggers的既有 workflow,而這個名稱不在ReservedSlugs(WorkflowService.cs:2236)裡。 - 修法:改成以 id 為鍵的子路由;或者把它加入保留字,並為既有資料重新產生 slug。
- 問題:新增的
合併前:匿名入口加固
- 先讀 body 才驗證
- 問題:controller 在查 slug、驗 key 之前,就先讀入最多 25 MiB 的 body(
MaxDeliveryBodyBytes,ExternalTriggersController.cs:34)。實測對不存在的 slug 送一個 24 MiB、未帶憑證的 POST,配置了約 177–341 MiB。對照之下,不存在的 webhook id 會先回 404,配置不到 1 MiB。 - 修法:先解析 slug、驗證 key,再用每個 trigger 自己的小上限讀 body。
- 問題:controller 在查 slug、驗 key 之前,就先讀入最多 25 MiB 的 body(
- 比對 key 前做了太多工作
- 問題:
ExternalTriggerIngress.cs:36-63在比對 key 之前,先做了 3 次查詢並反序列化整份 definition。 - 修法:查到 slug 列後立即比對 digest;不存在的 slug 與錯誤的 key 回應要一致。input schema 存到 trigger 列上,不要每次請求都解析整份定義。
- 問題:
- 限流
- 問題:
{slug}沒有長度限制,partition key 的大小由攻擊者決定。失敗的認證也會消耗合法呼叫方的額度:打 2000 次錯 key 之後,真 key 會拿到 429。 - 修法:失敗只計入位址桶;429 回應補上
Retry-After。
- 問題:
- 輸入校驗器沒有上限
- 問題:
ExternalTriggerPayloadValidator不設上限。3.8 MiB 的1,會產生約 200 萬筆錯誤、配置約 0.8 GiB;超長的整數指數會走進BigInteger.Parse,耗掉數秒 CPU。另外pattern、minimum、oneOf、$ref,甚至拼錯的type,目前都被靜默接受。 - 修法:錯誤數設上限(約 20 筆),不回顯原值;拒絕超長的數字 token;存檔時拒絕不支援的 schema 關鍵字。
- 問題:
- jsonb 存不下的合法 JSON 會回 500
- 問題:
\u0000、孤立 surrogate、1e200000都能通過解析,但存進 jsonb 時失敗,回 500。 - 修法:在入口直接回 400。
- 問題:
- 沒有 per-trigger 的 run 速率或併發上限
- 問題:一把外洩的 key,每分鐘可以用發布者的 authority 起約 2000 個 run。
- 修法:在
RunStarter之前加一個 admission 步驟,使用 committed 預設值。
後續 PR
- 分層與規範:Rule 16/18.1:Reveal 與 List handler 直接注入
CodeSpaceDbContext;Core 的 outcome 型別重複了 Messages 的 DTO,還需要一個 mapper。Rule 3/4:拆分ReceiveAsync(約 90 行)。Rule 1:建構子參數過多。 - reveal、rotate、rename:reveal 改成 POST,並加
Cache-Control: private, no-store。rotate、rename 要寫稽核 log,並加上並行控制。 - 三份真相:node、前端派生的 activation、trigger 列三者各自存在。透過 API 建立、卻沒有 activation 列的 workflow 會永遠 404。改用 node-save hook,也能解除
WorkflowService對trigger.external的耦合。 - payload 的信任邊界:發送端的 payload 原樣存入,Viewer 可讀。可考慮對選定欄位 seal、設定
SourceInstanceId/ExternalEventId,並在 goal template 中把這些內容標記為不受信任。 - 測試:補上 payload → run → engine 的端到端覆蓋,以及真 HTTP 的並行同 key、投遞/存檔/再投遞、
/api/ext的 429 情境。本機 mutation 測試中,E2E 層 15 個 mutant 一個都沒殺掉。
長期方向
改成以 registry 為基礎的 inbound endpoint:authenticator、body decoder、admission、responder 各自是 sibling interface,做法與既有的 webhook verifier/normalizer registry 相同。這樣 HMAC 發送端和其他 body 形式就能直接接入,不用改 controller。這部分可以等上面的契約定案後再做。
|
感谢详细 review。这轮按两部分处理: 已处理并验证
针对性验证结果:
部分处理,仍有遗留
|
ppXD
left a comment
There was a problem hiding this comment.
感謝這一輪修正。f60497bd8、0003bcf29、03c9cba0b 處理掉了前次審查裡最重的幾項:?key= 已移除、body 改為認證後才讀、拒絕狀態碼與稽核已分流且有界、冪等範圍改用穩定的 trigger id、external-triggers 的路由遮蔽連同既有資料一併修復,測試也補得很扎實。
在 03c9cba0b 上重新驗證後,還有七項建議在合併前處理。前四項會固定成公開契約,合併後很難再改。
合併前
-
URL 身分(前次 M2):已確定方向,改用伺服器產生的隨機 id。 目前是全域的人類命名 slug 加單一 key,實測有四個問題:
- 其他 team 建同名 slug 會得到 409,而且回應會回顯 slug;
- live slug 回 401、未知 slug 回 404,可以枚舉;
- rename 後,舊 URL 立即被別的 team 認領;
- rotate 會讓舊 key 立即失效。
請改成:
- 端點身分由伺服器產生,約 22 個 base64url 字元、128 位元隨機;路由改為
{id:length(22)}。 - 刪除 rename 命令、
config.slug、slug 輸入與 409 衝突路徑;編輯器改為唯讀顯示完整 URL。 - 0251 尚未合併,直接修改即可。日後若需要好記的名稱,再另加可為 null 的 per-team label,屬純加法。
- 現有測試裡把 rename 後立即失效釘成預期的部分,一併調整。
key slot 與可設定寬限的 rotate 可以列為後續;在那之前,請在文件寫明「rotate 立即生效」。
-
Body 上限。 目前沿用 webhook 的 25 MiB 並整包緩衝。認證後的 24 MiB 請求約配置 270 到 390 MiB。建議此端點使用自己的常數(1 MiB,最多 5 MiB)並直接從 stream 解析;上限日後調低會打斷已上線的發送端。
-
冪等與 admission 語意。 (a) 同 key 不同 body 目前回 200 deduplicated 並丟掉第二份 payload,建議比對既有 run 的 payload,不一致回 422;(b)
TryAcquire在StartAsync之前,重放會消耗 permit,之後新事件甚至已送達事件的重放都回 429,建議先查冪等,只在真的建立新 run 時才取 permit。 -
回應本體。 403/422 目前把例外原文回給外部呼叫者(含
membership-revoked、ModeProfileRegistry),且沒有穩定 code。建議回{code, message},訊息使用固定公開文字,原因只寫進稽核列。reveal 與 rotate 回應加Cache-Control: no-store,限流中介層的 429 補Retry-After。 -
認證前的成本。 key 比對仍在 3 次查詢與
WorkflowDefinition反序列化之後,且VerifyExternalTriggerCommand是ICommand,401/404 也開交易;成功請求還驗證兩次。1.5 MB 的 definition 下,錯 key 約 14 ms、8 MiB 每次,未知 slug 約 1 ms、0.13 MiB。key_digest已在 trigger 列上,digest 優先不需要把 schema 放進該列,所以不會形成第二個真相來源:認證階段只選id, team_id, workflow_id, node_id, key_digest,改為非交易式並對所有失敗回同一個 401;通過認證且讀完 body 後,才一次載入 activation、workflow 與版本,並直接沿用第一階段的結果。 -
save-time 白名單的 regression。 白名單擋掉了
x-long、x-hidden、x-selector,這三個是buildFieldSchema為 Paragraph、Hidden、Repository、Conversation 欄位產生的關鍵字,存檔回 422(inputFieldSchema.ts第 77、79、90 行)。建議對x-前綴放行,並加一個把每種欄位類型輸出都餵進ValidateSchema的契約測試。順帶建議檢查關鍵字的值形狀(required為字串陣列、enum非空陣列、maxLength非負 int32、根為type: object);目前required:"service"可存檔但不被強制執行,maxLength:"3"則讓之後每個請求都回 400。 -
payload 可表示性。 (a)
\u0000(值、屬性名稱)與屬性名稱中的 lone surrogate 仍回 500,發送端會重試且冪等無效;(b)IsInteger對無上限指數做BigInteger.Parse,256 字元上限只擋了double.TryParse。宣告 integer 欄位後,約 4 MB 的 body 實測約 3.9 秒 CPU,約 6 MiB 約 7 秒;(c)1e-16383這類 9 bytes 的數字能通過驗證,讀出時展開成約 16 KB,18 KB 的 body 使GET /api/workflows/runs/{id}回約 32.8 MB。建議在ValidateRepresentable一併走訪屬性名稱、拒絕 U+0000、拒絕指數絕對值大於約 330 的 token、token 超長時在IsInteger之前就返回,並在暫存步驟把PostgresException22xxx 對應成 400。順手把兩個 pass 的節點預算拆開(目前共用,宣告items的陣列實際上限約 4,999 而非文件的 10,000)。
之後追蹤即可(建議開 issue)
- key slot 與可設定寬限的 rotate、reveal 改 POST、rotate 與 rename 的併發保護(
ExecuteUpdate ... WHERE deleted_date IS NULL)、WWW-Authenticate。 - Bearer 優先於
X-Api-Key的行為與 authenticator 抽象。 - payload 密封、
SourceInstanceId、ExternalEventId與 untrusted 標記;文件先寫明不要在 body 放秘密。 - 有
trigger.external節點但無 activation 時的診斷、inputSchema與definition.Inputs的投影漂移。 - reveal 與 list handler 注入
CodeSpaceDbContext、Core 資料 record 與鏡像 outcome 型別(Rule 16/18)。 - 跨 replica 的 admission 計數、併發 run 上限、limiter 淘汰、錯誤回顯截斷。
- 0252 測試加 drift detector 與碰撞案例、列舉字面 GET 對照
ReservedSlugs的守門測試、An_unknown_slug_is_404_without_reading_the_body改為真正斷言 body 未被讀取。
第 1 到 7 項處理後即可合併,屆時只需核對對應的測試,不必再做一輪完整審查。
|
已将本轮加固提交
本轮新鲜验证:定向后端单测 107/107、集成测试 93/93、HTTP E2E 40/40、前端测试 2,821/2,821,前端 build 成功,lint 0 errors(3 条既有 warnings)。后端全量单测曾无进展后中止,因此这里仅报告定向测试结果。 |
Summary
工作流支持通过稳定的 External API 触发器端点接收 JSON,并启动对应 workflow run。本轮将端点身份、匿名认证、输入边界、幂等、容量 admission、公开错误响应和 schema 契约收敛为可验证的跨模块契约。
触发器端点使用服务端生成的 UUID,节点配置不再保存或派生 slug,也不再提供 rename。节点级 key 仍是唯一凭据;工作流保存、节点替换和 activation 重建不会改变存活节点的 trigger 身份,退役后旧 key 立即失效。
Related
Closes #2137
Changes
config.slug和 rename API。Authorization: Bearer或X-Api-Key,未知、退役和错误凭据统一返回固定的 401 响应。x-编辑器注解。Idempotency-Key按 trigger UUID 隔离;同 key 同内容返回已有 run,不同内容返回 422,重放不消耗新 run admission 容量。Retry-After,key 响应使用Cache-Control: private, no-store。inputSchema为唯一声明源,并通过前后端 schema contract 测试验证字段投影与运行时校验的一致性。Risk / Impact
Test plan