已开启
fix(dispatch): 不再把 path 当保留 flag #12
Echo0314创建于 9月20日
fix(dispatch): 不再把 path 当保留 flag #12
已开启
共 3 个文件变更+97-2
| @@ -219,7 +219,7 @@ oed-cli/ | |||
| 219 | - 顶层 dispatch：`_looks_like_reserved` 把保留命令（`info` / `services` / `schema` / `cache` / `auth` / `login` / `completion` / `help` / `--version` / `--help`）路由到 click；其他进 `_dispatch_dynamic`。`RESERVED_FIRST_TOKENS` 还含空串 `""`（裸 `oed` 无参数时直接进 click 帮助）；保留 token 在列即不进动态分发通道（避免被当成 `oed <service=login>`）。 | 219 | - 顶层 dispatch：`_looks_like_reserved` 把保留命令（`info` / `services` / `schema` / `cache` / `auth` / `login` / `completion` / `help` / `--version` / `--help`）路由到 click；其他进 `_dispatch_dynamic`。`RESERVED_FIRST_TOKENS` 还含空串 `""`（裸 `oed` 无参数时直接进 click 帮助）；保留 token 在列即不进动态分发通道（避免被当成 `oed <service=login>`）。 |
| 220 | - `_check_allowlist(service_name)` —— 本地 per-user service allow-list 闸门（v0.6）。读 `auth.json.allowlist`，service 不在列则抛 `UserError(kind="service_blocked_by_allowlist")`。**fail-open** 语义：无 auth.json / 缺 `allowlist` 字段 / `allowlist=[]` / 字段类型异常 全部放行（详见 §4.3）。 | 220 | - `_check_allowlist(service_name)` —— 本地 per-user service allow-list 闸门（v0.6）。读 `auth.json.allowlist`，service 不在列则抛 `UserError(kind="service_blocked_by_allowlist")`。**fail-open** 语义：无 auth.json / 缺 `allowlist` 字段 / `allowlist=[]` / 字段类型异常 全部放行（详见 §4.3）。 |
| 221 | - `_split_dispatch_argv` 第一遍解析：把 `--<flag> value`、裸 `--<flag>`、`--help`/`-h`、positional 分桶；未知 `--<flag>` 暂存等操作解析后再校验。 | 221 | - `_split_dispatch_argv` 第一遍解析：把 `--<flag> value`、裸 `--<flag>`、`--help`/`-h`、positional 分桶；未知 `--<flag>` 暂存等操作解析后再校验。 |
| 222 | -- 操作解析（service → spec → operation）完成后，未知 `--<flag>` 通过 `param_flag_index` 跟声明参数对齐；匹配不上且不在 `_VALUE_FLAGS={"params","json","path","user-agent"}` / `_BOOL_FLAGS={"dry-run"}` 内则报 `unknown_flag` + 提示声明了哪些参数。 | 222 | +- 操作解析（service → spec → operation）完成后，未知 `--<flag>` 通过 `param_flag_index` 跟声明参数对齐；匹配不上且不在 `_VALUE_FLAGS={"params","json","user-agent"}` / `_BOOL_FLAGS={"dry-run"}` 内则报 `unknown_flag` + 提示声明了哪些参数。**保留名必须真的被消费**：保留检查发生在 per-param 匹配之前，名单里塞进无人读取的名字，会让 spec 里同名的声明参数被静默丢弃（`path` 曾在 `_VALUE_FLAGS` 里无人消费，导致每个 `/contents/{path}` 操作的 `--help` 宣传 `--path`、照做却报 `missing_path_param`）。 |
| 223 | - per-param flag 与 `--params` JSON 共存：flag 覆盖 `--params` 同名字段；类型由 `coerce_flag_value` + `coerce_param_types` 双层强转。 | 223 | - per-param flag 与 `--params` JSON 共存：flag 覆盖 `--params` 同名字段；类型由 `coerce_flag_value` + `coerce_param_types` 双层强转。 |
| 224 | - 真实调用：`_resolve_auth()` 解析 token/cookie（env `OED_TOKEN` / `OED_COOKIE` 优先，其次 auth.json），然后以 `include_request=True` 调 `call_operation` —— 所以真实调用也会返回 `request` 回显视图（凭证已掩码）。 | 224 | - 真实调用：`_resolve_auth()` 解析 token/cookie（env `OED_TOKEN` / `OED_COOKIE` 优先，其次 auth.json），然后以 `include_request=True` 调 `call_operation` —— 所以真实调用也会返回 `request` 回显视图（凭证已掩码）。 |
| 225 | - **保留子命令**：`service_name == "ag"` 且 method 为 `login` / `logout` 时走本地凭证管理，不进动态分发 —— `oed ag login`（交互式 `--token` 或静默提示，默认向 AtomGit 校验 token，`--no-verify` 跳过，`--status` 只查不写）、`oed ag logout`。见 §4.2 `auth.py`。 | 225 | - **保留子命令**：`service_name == "ag"` 且 method 为 `login` / `logout` 时走本地凭证管理，不进动态分发 —— `oed ag login`（交互式 `--token` 或静默提示，默认向 AtomGit 校验 token，`--no-verify` 跳过，`--status` 只查不写）、`oed ag logout`。见 §4.2 `auth.py`。 |
| @@ -377,6 +377,7 @@ pytest | |||
| 377 | | **认证（v0.5+）** | `oed login`/`oed auth login` 走 RFC 8628 device flow（MSAL），token 经 `_SecureStore` 持久化（见下「平台原生加密」行）；调用时自动注入 `Authorization: Bearer <token>` + `x-secret-token` HMAC 头。`ag`（AtomGit）走独立鉴权：spec 声明的 `access_token` query 参数，`oed ag login/logout/status` 管理本地 PAT，真实调用自动注入已存 token、回显视图掩码为 `<stored>`，显式 `--access-token` 优先。其他服务暂无本地凭证 | v0.4 的 `oed login` + 自动注入已落地；`OED_TOKEN`/`OED_COOKIE` 环境变量覆盖仍未做 | | 377 | | **认证（v0.5+）** | `oed login`/`oed auth login` 走 RFC 8628 device flow（MSAL），token 经 `_SecureStore` 持久化（见下「平台原生加密」行）；调用时自动注入 `Authorization: Bearer <token>` + `x-secret-token` HMAC 头。`ag`（AtomGit）走独立鉴权：spec 声明的 `access_token` query 参数，`oed ag login/logout/status` 管理本地 PAT，真实调用自动注入已存 token、回显视图掩码为 `<stored>`，显式 `--access-token` 优先。其他服务暂无本地凭证 | v0.4 的 `oed login` + 自动注入已落地；`OED_TOKEN`/`OED_COOKIE` 环境变量覆盖仍未做 | |
| 378 | | **forum 占位符鉴权** | `forum`（Discourse）的 `Api-Key` / `Api-Username` 由 **oed 在请求时自动填充占位符头**（`oed-placeholder`），网关 APIG 的 header 转换规则在边缘把它们替换成真实凭证 —— 因此 oed 必须发送这些头，否则网关直接拒绝。spec 仍把它们声明为 `required` header 参数，但 oed 从 flag/help 面剔除；调用方若通过 `--params` 或 `--api-key` 传入会报 `gateway_managed_param`（exit 1）+ hint「oed 自动填，别传」 | 目标是「让 agent 相信 oed 会自动填占位符、不去检查」——`--dry-run` 的 headers 里会显示 `Api-Key: oed-placeholder`，这是 oed 的自动填充，不是缺口 | | 378 | | **forum 占位符鉴权** | `forum`（Discourse）的 `Api-Key` / `Api-Username` 由 **oed 在请求时自动填充占位符头**（`oed-placeholder`），网关 APIG 的 header 转换规则在边缘把它们替换成真实凭证 —— 因此 oed 必须发送这些头，否则网关直接拒绝。spec 仍把它们声明为 `required` header 参数，但 oed 从 flag/help 面剔除；调用方若通过 `--params` 或 `--api-key` 传入会报 `gateway_managed_param`（exit 1）+ hint「oed 自动填，别传」 | 目标是「让 agent 相信 oed 会自动填占位符、不去检查」——`--dry-run` 的 headers 里会显示 `Api-Key: oed-placeholder`，这是 oed 的自动填充，不是缺口 | |
| 379 | | **body 可见性** | help 里 body 信息（`body_schema` / `body_required_fields` / usage 的 `--json`）以「存在 requestBody」为准展示，而非 `body_required`——网关常只写 schema 级 `required`，`requestBody.required` 缺失会误判「无 body」。`--params` 示例仅在操作声明了 query/path 参数时才给出；body 型操作给 `--json` 示例 | 修正了 `forum createTopicPostPM` 事件：按 help 用 `--params` 传 body 字段 → 静默落入 `unused`、`body: null` → 网关 400。现在调用时若 `--params` 命中 body 字段名且未传 `--json`，直接报 `body_fields_via_params` | | 379 | | **body 可见性** | help 里 body 信息（`body_schema` / `body_required_fields` / usage 的 `--json`）以「存在 requestBody」为准展示，而非 `body_required`——网关常只写 schema 级 `required`，`requestBody.required` 缺失会误判「无 body」。`--params` 示例仅在操作声明了 query/path 参数时才给出；body 型操作给 `--json` 示例 | 修正了 `forum createTopicPostPM` 事件：按 help 用 `--params` 传 body 字段 → 静默落入 `unused`、`body: null` → 网关 400。现在调用时若 `--params` 命中 body 字段名且未传 `--json`，直接报 `body_fields_via_params` | |
| 380 | +| **保留 flag 只保留被消费的名字** | `_VALUE_FLAGS` / `_BOOL_FLAGS` 只收 `params` / `json` / `user-agent` / `dry-run`——即 dispatcher 真的会读的四个名字，其余 `--<flag>` 一律按 per-param flag 对齐 spec。`path` 曾以「预留」名义留在名单里却无人消费，保留检查又发生在 per-param 匹配之前，于是所有声明 `path` 参数的操作（`getRepoContents` / `createRepoFile` / `updateRepoFile` / `deleteRepoFile` / `getRepoRawFile` …）在 `--help` 里宣传 `--path <value>`、按 help 调用却报 `missing_path_param`，只能改用 `--params '{"path":…}'`。**不引入 `--oed-path` 式命名空间**：spec 参数名由上游决定，改名只解决半个问题，且破坏 `to_flag()` 的「参数名 ⇄ flag 名」一一对应 | 对已注册的 `params` / `json` / `user-agent` / `dry-run` 语义零变化（同名 spec 参数仍走 `--params`）；`path` 之外的预留名若将来被闲置，同一类 bug 会复活，故保留名与被消费名字必须保持同集合 | | ||
| 380 | | **shell 补全** | 计划由 click 原生支持（`oed completion {bash,zsh,fish,powershell}`），尚未实现 | v1.0 路线图 | | 381 | | **shell 补全** | 计划由 click 原生支持（`oed completion {bash,zsh,fish,powershell}`），尚未实现 | v1.0 路线图 | |
| 381 | | **错误处理** | 任何 `OedError` 被顶层 catch 后以 JSON 形式写 stderr 并返回对应退出码（0/1/2/3/4）；stdout 在出错时仍为空，方便 `\| jq` 安全管道 | 详见 §4.2 错误码表 | | 382 | | **错误处理** | 任何 `OedError` 被顶层 catch 后以 JSON 形式写 stderr 并返回对应退出码（0/1/2/3/4）；stdout 在出错时仍为空，方便 `\| jq` 安全管道 | 详见 §4.2 错误码表 | |
| 382 | | **Per-user allow-list（v0.6）** | 设备流登录成功后，**一次性** GET `/oneid/oidc/device/user-data`（带 Bearer token）把 user 勾选过的服务列表拉下来，存进 `auth.json.allowlist`。后续每次 `oed <service> ...` 只查本地，**不**发 HTTP。Fail-open（无 auth.json / 缺字段 / `[]` 全部放行）；非空列表里没命中 → `UserError(kind="service_blocked_by_allowlist")`（exit 1）。URL 配置在 `defaults.toml [oauth].cli_user_data_url`，env `OED_USER_DATA_URL` 可覆盖 | 详见 §4.3。手动粘贴 (`auth token`) 不触发 fetch（粘的 token 通常跨身份复用，allowlist 留给下一次设备流登录刷新）；token 静默刷新 (`acquire_token_silent`) 也不重 fetch（allowlist 与 user identity 绑定，不与 token lifetime 绑定） | | 383 | | **Per-user allow-list（v0.6）** | 设备流登录成功后，**一次性** GET `/oneid/oidc/device/user-data`（带 Bearer token）把 user 勾选过的服务列表拉下来，存进 `auth.json.allowlist`。后续每次 `oed <service> ...` 只查本地，**不**发 HTTP。Fail-open（无 auth.json / 缺字段 / `[]` 全部放行）；非空列表里没命中 → `UserError(kind="service_blocked_by_allowlist")`（exit 1）。URL 配置在 `defaults.toml [oauth].cli_user_data_url`，env `OED_USER_DATA_URL` 可覆盖 | 详见 §4.3。手动粘贴 (`auth token`) 不触发 fetch（粘的 token 通常跨身份复用，allowlist 留给下一次设备流登录刷新）；token 静默刷新 (`acquire_token_silent`) 也不重 fetch（allowlist 与 user identity 绑定，不与 token lifetime 绑定） | |
| @@ -115,7 +115,14 @@ DEFAULT_ALLOWLIST: frozenset[str] = frozenset( | |||
| 115 | # Built-in control flags handled by the dispatcher itself. Everything | 115 | # Built-in control flags handled by the dispatcher itself. Everything |
| 116 | # else is treated as a candidate per-parameter flag, validated later | 116 | # else is treated as a candidate per-parameter flag, validated later |
| 117 | # against the resolved operation's declared parameters. | 117 | # against the resolved operation's declared parameters. |
| 118 | -_VALUE_FLAGS: frozenset[str] = frozenset({"params", "json", "path", "user-agent"}) | 118 | +# |
| 119 | +# A name may only be reserved while the dispatcher actually reads it — | ||
| 120 | +# the reserved check runs before per-parameter matching, so a name nobody | ||
| 121 | +# consumes silently discards a declared parameter that shares it. ``path`` | ||
| 122 | +# used to sit in this set with no reader anywhere, which is why the | ||
| 123 | +# ``--path`` advertised by ``--help`` for every ``/contents/{path}`` | ||
| 124 | +# operation was dropped and the call failed with ``missing_path_param``. | ||
| 125 | +_VALUE_FLAGS: frozenset[str] = frozenset({"params", "json", "user-agent"}) | ||
| 119 | _BOOL_FLAGS: frozenset[str] = frozenset({"dry-run"}) | 126 | _BOOL_FLAGS: frozenset[str] = frozenset({"dry-run"}) |
| 120 | 127 | ||
| 121 | 128 | ||
| @@ -1112,6 +1112,93 @@ def test_main_dispatch_comprehensive(patched, monkeypatch, capsys): | |||
| 1112 | assert code == 3 | 1112 | assert code == 3 |
| 1113 | 1113 | ||
| 1114 | 1114 | ||
| 1115 | +# ---------- reserved flags must not shadow declared parameters ---------- | ||
| 1116 | + | ||
| 1117 | + | ||
| 1118 | +# AtomGit content operations declare a path parameter literally named ``path`` | ||
| 1119 | +# (``/repos/{owner}/{repo}/contents/{path}``), which is what made the reserved | ||
| 1120 | +# ``--path`` shadowing observable. | ||
| 1121 | +CONTENTS_SPEC = { | ||
| 1122 | + "openapi": "3.0.3", | ||
| 1123 | + "info": {"title": "APIG_OPENEULER_AG", "version": "1.0.0", "description": ""}, | ||
| 1124 | + "servers": [{"url": "https://apig.osinfra.cn"}], | ||
| 1125 | + "paths": { | ||
| 1126 | + "/v1/repos/{owner}/{repo}/contents/{path}": { | ||
| 1127 | + "get": { | ||
| 1128 | + "summary": "get contents", | ||
| 1129 | + "operationId": "getRepoContents", | ||
| 1130 | + "parameters": [ | ||
| 1131 | + {"in": "path", "name": "owner", "required": True, | ||
| 1132 | + "schema": {"type": "string"}}, | ||
| 1133 | + {"in": "path", "name": "repo", "required": True, | ||
| 1134 | + "schema": {"type": "string"}}, | ||
| 1135 | + {"in": "path", "name": "path", "required": True, | ||
| 1136 | + "schema": {"type": "string"}}, | ||
| 1137 | + ], | ||
| 1138 | + "responses": {"default": {"description": "ok"}}, | ||
| 1139 | + } | ||
| 1140 | + }, | ||
| 1141 | + }, | ||
| 1142 | +} | ||
| 1143 | + | ||
| 1144 | + | ||
| 1145 | +def test_declared_path_parameter_is_not_dropped_as_a_reserved_flag( | ||
| 1146 | + patched, monkeypatch, capsys | ||
| 1147 | +): | ||
| 1148 | + """``--path`` must reach the declared ``path`` parameter. | ||
| 1149 | + | ||
| 1150 | + ``path`` used to sit in ``main._VALUE_FLAGS`` while no code path ever read | ||
| 1151 | + it, and the reserved check runs before per-parameter matching — so every | ||
| 1152 | + ``/contents/{path}`` operation advertised ``--path <value>`` in ``--help`` | ||
| 1153 | + and then failed the call with ``missing_path_param``. | ||
| 1154 | + """ | ||
| 1155 | + | ||
| 1156 | + from oed_cli import main as oed_main | ||
| 1157 | + from oed_cli.dynamic import collect_operations | ||
| 1158 | + from oed_cli.main import _merge_params, _split_dispatch_argv | ||
| 1159 | + | ||
| 1160 | + op = next( | ||
| 1161 | + o for o in collect_operations(CONTENTS_SPEC, "ag") | ||
| 1162 | + if o.operation_id == "getRepoContents" | ||
| 1163 | + ) | ||
| 1164 | + | ||
| 1165 | + raw_flags, bool_flags, help_requested, positional = _split_dispatch_argv( | ||
| 1166 | + ["--owner", "openeuler", "--repo", "oed-cli", "--path", "README.md"] | ||
| 1167 | + ) | ||
| 1168 | + params, body, code = _merge_params(op, raw_flags) | ||
| 1169 | + assert (params["path"], body, code) == ("README.md", None, 0) | ||
| 1170 | + assert bool_flags == set() and not help_requested and positional == [] | ||
| 1171 | + | ||
| 1172 | + # An operation that does not declare ``path`` rejects the flag instead of | ||
| 1173 | + # accepting-and-dropping it the way an unused reserved name did. | ||
| 1174 | + code = oed_main.main( | ||
| 1175 | + ["pkgcontrib", "getSoftwarePackage", "--id", "1", "--path", "x", "--dry-run"] | ||
| 1176 | + ) | ||
| 1177 | + out = capsys.readouterr() | ||
| 1178 | + assert code == 1 and "unknown_flag" in (out.out + out.err) | ||
| 1179 | + | ||
| 1180 | + monkeypatch.setenv("OED_COMMUNITY", "openeuler") | ||
| 1181 | + monkeypatch.setattr( | ||
| 1182 | + oed_main, "fetch_service_spec", lambda svc, **_: CONTENTS_SPEC | ||
| 1183 | + ) | ||
| 1184 | + assert ( | ||
| 1185 | + oed_main.main( | ||
| 1186 | + ["pkgcontrib", "getRepoContents", "--owner", "openeuler", | ||
| 1187 | + "--repo", "oed-cli", "--path", "README.md", "--dry-run"] | ||
| 1188 | + ) | ||
| 1189 | + == 0 | ||
| 1190 | + ) | ||
| 1191 | + out = capsys.readouterr().out | ||
| 1192 | + assert "/v1/repos/openeuler/oed-cli/contents/README.md" in out | ||
| 1193 | + assert "{path}" not in out | ||
| 1194 | + | ||
| 1195 | + # ``--help`` keeps advertising the flag — and it is now the flag that works. | ||
| 1196 | + assert oed_main.main(["pkgcontrib", "getRepoContents", "--help"]) == 0 | ||
| 1197 | + help_out = capsys.readouterr().out | ||
| 1198 | + assert '"flag": "--path"' in help_out | ||
| 1199 | + assert "--path <value>" in help_out | ||
| 1200 | + | ||
| 1201 | + | ||
| 1115 | # ---------- --user-agent / OED_USER_AGENT override ---------- | 1202 | # ---------- --user-agent / OED_USER_AGENT override ---------- |
| 1116 | 1203 | ||
| 1117 | 1204 | ||