feat(plugin-protocol): scope secret injection by endpoint - #30
Conversation
|
Blocking compatibility finding before this can be reviewed: Older schema-v2 consumers accept unknown I have converted this PR to draft while the rollout/negotiation design is clarified. We should not merge the optional fields as currently shaped. Possible directions need maintainer agreement (schema bump, a fail-closed capability marker understood by old validators, or distribution-level minimum-client gating). This also affects the parent Desktop PR, which will remain local until the compatibility contract is decided. |
|
| Filename | Overview |
|---|---|
| packages/plugin-protocol/src/manifest.ts | Adds schema v3 endpoint-scope types, strict pathname and method validation, deterministic normalization, and mixed-version fail-closed handling. |
| packages/plugin-protocol/src/tests/manifest.test.ts | Covers schema compatibility, endpoint-scope normalization, unsafe path rejection, supported methods, and legacy host-order preservation. |
| packages/plugin-protocol/src/tests/delivery.test.ts | Verifies that normalized delivery responses retain path and method restrictions. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Read plugin manifest] --> B{Schema version}
B -->|v2| C{paths or methods declared?}
C -->|Yes| D[Reject manifest]
C -->|No| E[Preserve host-only injection]
B -->|v3| F[Validate hosts, paths, and methods]
F -->|Invalid or ambiguous| D
F -->|Valid| G[Normalize endpoint allowlists]
G --> H[Deliver normalized manifest]
H --> I[Desktop enforces endpoint-scoped injection]
Reviews (6): Last reviewed commit: "feat(plugin-protocol): endpoint-scoped s..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24250110d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** 精确 URL.pathname 白名单；缺省 = 该 host 下全部路径。 */ | ||
| paths?: string[]; | ||
| /** HTTP method 白名单；缺省 = 代理 fetch 支持的全部方法。 */ | ||
| methods?: GhostFetchMethod[]; |
There was a problem hiding this comment.
Because these restrictions are added as optional fields without changing the schema version, any pre-change Desktop will accept a manifest containing them, treat paths and methods as unknown, and strip them during validateGhostManifest normalization. Its runtime will consequently interpret the secret as host-wide and may inject credentials into every path and method on that host. Require a schema/capability gate, or otherwise prevent endpoint-scoped manifests from reaching clients that do not enforce these fields.
Useful? React with 👍 / 👎.
| || pathname.includes('?') | ||
| || pathname.includes('#') | ||
| || pathname.includes('\\') | ||
| || /[ - ]/.test(pathname) |
There was a problem hiding this comment.
This regex contains a literal NUL byte and other raw control characters in the TypeScript source, causing byte-oriented text tools to classify the entire file as binary and omit its contents from normal searches or transformations. Spell the bounds using escaped code points such as \u0000-\u001f\u007f so the validator retains the same behavior without turning manifest.ts into a binary-looking file.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: SAN <fierushio@gmail.com>
…ation 上一提交把控制字符以字面字节写进了正则,导致 `pnpm lint` 报 no-control-regex、`pnpm format:check` 失败,并让 manifest.ts 在 file(1) / grep 等文本工具下被当成二进制。改用 \x 转义序列并按仓库 风格重排,行为不变。 同时补一条 delivery 层用例:endpoint 收窄必须穿过 release manifest 的重新校验与规范化。delivery.ts 是除 validateGhostManifest 之外的 第二个规范化边界,收窄一旦在这里被丢弃,市场安装路径就会退回 "整域可注入"。已用变异验证:去掉规范化输出里的 paths/methods 后, 该用例与 manifest 用例同时失败。 验证: - pnpm --filter @cindy/plugin-protocol test → 36 passed - pnpm --filter @cindy/plugin-protocol build (tsc --noEmit) → 通过 - eslint . → 无告警 - prettier --check . → 全部符合 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: SAN <fierushio@gmail.com>
4b59bc6 to
2984e69
Compare
与 paths / methods 同款排序归一化:host 顺序的无意义变动不再引起 权限展示与批准 diff 抖动。 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: SAN <fierushio@gmail.com>
manifestDigest 按数组原始顺序计算;对旧 host-only 清单排序会让已装 插件的账本摘要永久失配,触发市场所有权检查与 OIDC 签发拒绝。 改为仅当同一凭证声明 inject.paths / inject.methods 时排序 hosts, 此时权限 detail 才需要稳定输出;旧清单归一化结果与升级前逐字节一致。 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: SAN <fierushio@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b860a6a579
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /%(?:2f|5c)/i.test(pathname) || | ||
| /%(?![0-9a-f]{2})/i.test(pathname) |
There was a problem hiding this comment.
When a request passes through a proxy/framework pair that percent-decodes at both layers, this check is bypassed by a path such as /public%252f..%252fadmin: neither regexp matches and the WHATWG pathname remains unchanged, so the manifest is accepted, but two decodes produce /public/../admin. The credential can therefore reach a different endpoint than the permission text indicates; recursively validate decoding to a stable safe representation or reject encoded percent sequences that can become separators, traversal segments, or controls.
Useful? React with 👍 / 👎.
| /** 精确 URL.pathname 白名单；缺省 = 该 host 下全部路径。 */ | ||
| paths?: string[]; | ||
| /** HTTP method 白名单；缺省 = 代理 fetch 支持的全部方法。 */ | ||
| methods?: GhostFetchMethod[]; |
There was a problem hiding this comment.
For manifests using network.connections, placing the newly introduced paths or methods fields in connections[].inject is silently accepted by validateGhostManifest but discarded when the connection declaration is normalized, leaving that connection token injectable across every path and method on the instance. This is especially misleading because GhostConnectionDecl.inject is documented as using the same validation as GhostSecretInjectDecl; either preserve and enforce these scopes for connection credentials or explicitly reject them instead of failing open.
Useful? React with 👍 / 👎.
Signed-off-by: SAN <fierushio@gmail.com>
…xed-version fail-closed) v2 清单声明 inject.paths/methods 直接拒装:旧客户端不识别这两个字段, 放行会让收窄静默退化为整域注入(fail-open)。声明新字段即升级版本, 旧客户端对 v3 整包拒装(schemaVersion 严格相等检查),形成 fail-closed 边界。normalize 输出保留输入版本,v3 清单 manifestDigest 与打包时一致。 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: SAN <fierushio@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8192b56f00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| raw.schemaVersion !== GHOST_MANIFEST_SCHEMA_VERSION && | ||
| raw.schemaVersion !== GHOST_MANIFEST_SCHEMA_VERSION_ENDPOINT_SCOPE |
There was a problem hiding this comment.
Accepting schema version 3 here leaves the public protocol guide contradictory: docs/plugin-protocol.md still states that manifests only accept GHOST_MANIFEST_SCHEMA_VERSION=2 and that new optional fields do not require coordinated releases. Plugin authors following that contract will declare paths/methods under v2 and have their manifests rejected by this validator, so document the v3 requirement and endpoint-scope capability alongside this change.
Useful? React with 👍 / 👎.
8192b56 to
6d1a4f2
Compare
|
CI workflow 目前停在 如果方便的话,请在 Actions 页面为本次 run 批准执行:https://github.com/makecindy/cindy-protocol/actions/runs/31235885640 本地验证已全绿:17 个测试文件 274 测试通过、tsc/eslint/prettier 干净。 |
1 participant
Summary
inject.pathsexact pathname allowlistsinject.methodsallowlists for supported Ghost fetch methodsTracks makecindy/cindy#1714. This is the protocol-side prerequisite for the Desktop runtime change.
Validation
pnpm --filter @cindy/plugin-protocol test— 35 tests passedpnpm --filter @cindy/plugin-protocol build— passedThe repository-wide wrapper was attempted but Corepack failed while downloading pnpm 10.33.2 under Node 26 (
UND_ERR_INVALID_ARG); the affected package gates above passed directly.Compatibility and security
URL.pathnamevalues; query and fragment are excluded.🤖 Generated with Claude Code