Skip to content

feat(controller): allow L2 humans to update worker skills within their teams - #1212

Open
LUOSENGWA wants to merge 1 commit into
agentscope-ai:mainfrom
LUOSENGWA:feat/l2-worker-scoped-write
Open

feat(controller): allow L2 humans to update worker skills within their teams#1212
LUOSENGWA wants to merge 1 commit into
agentscope-ai:mainfrom
LUOSENGWA:feat/l2-worker-scoped-write

Conversation

@LUOSENGWA

@LUOSENGWA LUOSENGWA commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Worker skill/MCP self-service updates for team-scoped humans

Summary

PUT /api/v1/workers/{name} was only reachable by admins, managers, and team leaders. A team-scoped human (a Human CR with permissionLevel: 2) could read the workers in their accessibleTeams and drive projects there, but could not adjust the capabilities of the workers they coordinate: enabling a built-in skill, a remote registry skill, or an MCP server required escalating to an admin for every change.

This PR opens the existing update endpoint to that caller class with a code-level boundary:

  • Team scope — the worker must be a member of one of the caller's accessibleTeams (same team-membership lookup the list endpoints use). Out-of-scope workers — cross-team or standalone — are hidden from team-scoped humans on the update path the same way the read path hides them (404), so a scoped human cannot probe worker existence or learn a worker's owning team.
  • Field whitelist — only skills, remoteSkills, and mcpServers may be set. Any other field in the body (model, image, soul, agents, runtime, package, expose, channelPolicy, resources, containerManaged, state, …) is rejected with 400 naming the offending fields. Ownership, persona, image, network, and lifecycle remain the team owner's domain.

The layering matches the existing project write endpoints: the middleware cannot resolve worker -> team, so the authorizer change (ActionUpdate on worker for humans → requireSameTeam) is a pass-through for the scoped request, and the handler is the single enforcement point. The request type also gains remoteSkills, which the CRD and deployer already support but the API could not set even for admins.

What's included

  • internal/auth/authorizer.goActionUpdate on worker for team-scoped humans routes through requireSameTeam instead of a hard deny (teamless humans are still rejected at the middleware).
  • internal/server/resource_handler.goUpdateWorker enforces the boundary for human callers: team scope (404 cross-team and standalone, probe-resistant) + field whitelist (400, offending fields named). Applies remoteSkills for all callers.
  • internal/server/types.goUpdateWorkerRequest.remoteSkills.
  • docs/design/l2-worker-scoped-write.md — design contract.
  • docs/usage/resource-management.md (+ zh-cn) — role matrix for worker updates.

Data boundary

  • No new resources, no schema change, no CRD change. remoteSkills already exists in the Worker CRD and is consumed by the deployer's skill-push path; this PR only makes it settable through the API.
  • Purely additive for existing callers: admin/manager/team-leader behavior on PUT /api/v1/workers/{name} is unchanged (covered by regression tests).
  • The whitelist is enforced after JSON decode in the handler; a rejected body is written back untouched (no partial application — the check runs before the K8s update loop).
  • Rejected probes: out-of-scope worker (cross-team or standalone) → 404 (never 403), so the endpoint does not reveal worker names or team ownership to team-scoped humans.

Tests

  • internal/auth/authorizer_test.goTestAuthorizer_HumanScoped: in-scope and unresolved-team update worker allowed at the authorizer layer, cross-team denied; create/delete/wake/sleep on workers still denied.
  • internal/server/resource_handler_l2_update_test.go (new, 8 cases) — in-scope skills/remoteSkills/mcpServers update applies and returns 200; cross-team worker 404; standalone worker 404; off-whitelist fields 400 with field names; empty body no-op 200; admin full update unchanged; team leader update unchanged; teamless human hidden (404 at the handler; the middleware rejects it first).
  • go test ./internal/server/ ./internal/auth/ green; full go test ./... run: 19 packages ok, the only failure is a pre-existing environment issue in internal/executor (unzip binary absent in the test container) untouched by this diff. gofmt/go vet clean.

Related


团队人类用户的 Worker 技能/MCP 自助更新

摘要

PUT /api/v1/workers/{name} 此前只有 admin、manager、团队 Leader 可调。团队范围的人类用户(permissionLevel: 2 的 Human CR)能读到自己 accessibleTeams 内的 worker、能驱动项目,却无法调整自己协调的 worker 的能力配置——启用一个内置技能、远程注册表技能或 MCP 服务器,每次都要升级到 admin。

本 PR 在现有更新端点上为该角色打开入口,代码级双层边界:

  • 团队范围 — worker 必须是调用者 accessibleTeams 内某团队的成员(与列表端点同一套成员查询)。越权 worker(跨团队或独立)写路径与读路径同样隐藏(404),L2 无法探测 worker 存在性或所属团队,端点保持防探测。
  • 字段白名单 — 只能设置 skillsremoteSkillsmcpServers。body 触碰任何其他字段(modelimagesoulagentsruntimepackageexposechannelPolicyresourcescontainerManagedstate 等)即 400 拒绝并点名。归属、人设、镜像、网络、生命周期仍属团队所有者权限。

分层与既有项目写端点一致:中间件无法解析 worker -> team,authorizer 改动(人类对 worker 的 ActionUpdaterequireSameTeam)对无团队上下文的请求是放行,真正强制点在 handler。请求结构体顺带补上 remoteSkills——CRD 和 deployer 早已支持,但 API 此前连 admin 都改不了。

包含内容

  • internal/auth/authorizer.go — 人类对 worker 的 ActionUpdate 改走 requireSameTeam(无团队的 L2 仍在中间件被拒)。
  • internal/server/resource_handler.goUpdateWorker 对人类调用方强制边界:团队范围(跨团队与独立 worker 均 404,防探测)+ 字段白名单(400 点名)。对全部调用方应用 remoteSkills
  • internal/server/types.goUpdateWorkerRequest.remoteSkills
  • docs/design/l2-worker-scoped-write.md — 设计契约。
  • docs/usage/resource-management.md(+ zh-cn)— Worker 更新的角色权限矩阵。

数据边界

  • 无新资源、无 schema 变更、无 CRD 变更。remoteSkills 在 Worker CRD 中已存在且被 deployer 的技能推送链路消费,本 PR 只是让它可通过 API 设置。
  • 对既有调用方纯增量:admin/manager/团队 Leader 的 PUT /api/v1/workers/{name} 行为不变(有回归测试覆盖)。
  • 白名单在 JSON 解码后、K8s 更新循环前检查——被拒 body 原样写回,无部分应用。
  • 防探测:越权 worker(跨团队或独立)返回 404(而非 403),不向 L2 泄露 worker 名称或团队归属。

测试

  • internal/auth/authorizer_test.goTestAuthorizer_HumanScoped:本团队与无团队上下文的 update worker 在 authorizer 层放行,跨团队拒绝;worker 的 create/delete/wake/sleep 仍拒绝。
  • internal/server/resource_handler_l2_update_test.go(新,8 用例)— 本团队 skills/remoteSkills/mcpServers 更新生效返回 200;跨团队 worker 404;独立 worker 404;白名单外字段 400 且点名;空 body 无操作 200;admin 全字段更新不变;团队 Leader 更新行为不变;无团队 L2 隐藏(handler 404,中间件先行拒绝)。
  • go test ./internal/server/ ./internal/auth/ 全绿;全量 go test ./...:19 包 ok,唯一失败为 internal/executor 的预存环境问题(测试容器缺 unzip),与本 diff 无关。gofmt/go vet 干净。

相关

  • 扩展项目写端点(create/pause/resume/replan/cancel/complete)确立的团队范围模式:handler 是团队范围调用方的强制点。
  • 设计文档:docs/design/l2-worker-scoped-write.md
  • Related design: Design: L2 permission & capability model (worker config, channels, approval, skill catalog) #1220 (设计:L2 权限与 capability 模型(worker 配置、频道、审批、技能目录) — this PR's non-sensitive allowlist + write-only secret semantics implement that design.

@shiyiyue1102 shiyiyue1102 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on L2 self-service management. Allowing L2 users to view and edit non-sensitive Worker, Skill, and MCP configuration within their own teams is a reasonable and valuable direction. The concern is not that these operations must remain admin-only, but that the current permission boundary treats every L2 user and every configuration field in the same way.

Please separate the permission model into at least two levels:

  1. By default, L2 users may view and edit non-sensitive configuration for Workers in their accessibleTeams.
  2. Sensitive operations and data require an explicit FullAccess-like capability granted by an admin. This includes revealing credentials, managing arbitrary external Skill sources, and binding credentials to external endpoints.

Editing an MCP address itself can remain a normal L2 capability, but the Worker Gateway bearer token must not automatically be attached to an arbitrary user-provided URL. It should only be attached to trusted Gateway endpoints, or credentials should be managed through a separate credentialRef.

Similarly, assigning a Skill from the public catalog can be a default L2 capability, while adding an arbitrary remote registry and accessing its credentials should require the elevated permission.

Please avoid using the generic Worker ActionUpdate as the permission boundary for all these capabilities. Separate actions make it possible to authorize normal configuration, external sources, and sensitive access independently.

Before continuing with the implementation, please open a GitHub Issue describing the L2 permission matrix, sensitive-field classification, FullAccess assignment model, and credential handling. Link the Issue to this PR so other community members can participate in the design discussion.


感谢补充 L2 自助管理能力。允许 L2 用户在自己的 accessibleTeams 范围内查看和编辑非敏感的 Worker、Skill 和 MCP 配置,是合理且有价值的方向。当前问题并不是这些操作必须继续由 Admin 独占,而是现有权限边界把所有 L2 用户和所有配置字段都按同一种权限处理了。

建议至少拆分成两级权限:

  1. L2 默认可以查看和编辑本团队 Worker 的非敏感配置。
  2. 敏感操作和数据需要由 Admin 显式授予类似 FullAccess 的能力,包括查看凭据、管理任意外部 Skill Source,以及向外部地址绑定凭据。

编辑 MCP 地址本身可以作为普通 L2 能力,但不能把 Worker Gateway Bearer Token 自动附加到任意用户填写的 URL。Bearer Token 只能发送给可信 Gateway 地址,或者通过独立的 credentialRef 管理。

同样,从公共 Catalog 为 Worker 分配 Skill 可以默认开放给 L2;新增任意远程注册表以及访问其凭据,则应要求更高权限。

请不要使用通用的 Worker ActionUpdate 统一承载这些能力。应将普通配置、外部数据源和敏感数据访问拆成独立 Action,才能分别授权。

继续实现前,请创建一个 GitHub Issue,说明 L2 权限矩阵、敏感字段分类、FullAccess 分配方式和凭据处理规则,并关联到本 PR,让社区其他同学也能参与设计讨论。

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Adds a code-level L2-human boundary on PUT /api/v1/workers/{name}: team-scoped access (404 cross-team/standalone, probe-resistant) plus a field whitelist limited to skills/remoteSkills/mcpServers, with authorizer pass-through and thorough handler tests. The mechanics are well implemented, but the maintainer's outstanding CHANGES_REQUESTED design review (permission levels, sensitive-field classification, separate actions instead of generic ActionUpdate, design Issue first) is unresolved, and our code-level check confirms the credential concern behind it is concrete.

Findings

  • [Critical] resource_handler.go:257-262 — the whitelist permits L2-set mcpServers, and agentconfig.GenerateMcporterConfig injects Authorization: Bearer <gatewayKey> into every MCP entry using the URL verbatim, so an L2 human can currently point a worker MCP entry at an arbitrary URL and receive the gateway consumer key there (credential exfiltration path). Same class of risk applies to arbitrary remoteSkills registries with credentials.
  • [Warning] resource_handler.go:896 — the forbidden-field list is fail-open: future fields added to UpdateWorkerRequest become L2-writable by default; suggest a reflection-based sync test or inverting to an allowlist.
  • [Info] The whitelist is complete against the current UpdateWorkerRequest struct (all 13 non-whitelisted fields covered), the 404-vs-403 probe-resistance matches the read path, and the authorizer/handler layering is consistent with the project-write pattern — good.

Suggestions

Holding off on approval pending the design discussion requested by the maintainer (L2 permission matrix Issue + action separation). The inline findings are meant as concrete evidence for that discussion, not as a separate approval path.


Automated review by github-manager-bot

if req.Skills != nil {
worker.Spec.Skills = req.Skills
}
if req.RemoteSkills != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L2-controlled remoteSkills / mcpServers reach the worker verbatim. Note that agentconfig.GenerateMcporterConfig (internal/agentconfig/mcporter.go) uses each MCP server URL as-is and unconditionally injects Authorization: Bearer <gatewayKey> — the same consumer key used for LLM access. With this PR an L2 human can register an MCP entry pointing at an arbitrary external URL, so the worker would deliver the gateway consumer key to an attacker-controlled endpoint (credential exfiltration). This matches the maintainer's review: bearer attachment must be restricted to trusted gateway endpoints or moved to an explicit credentialRef, and arbitrary remote skill registries should require an elevated (FullAccess-like) permission.

// may only touch the self-service fields (skills / remoteSkills /
// mcpServers); everything else (model, image, identity, resources, ...) is
// the team owner's domain. Returns (0, "") when the update is allowed.
func (h *ResourceHandler) checkHumanWorkerUpdate(ctx context.Context, caller *authpkg.CallerIdentity, name string, req *UpdateWorkerRequest) (int, string) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field policy here is fail-open: it enumerates the forbidden fields, so any field added to UpdateWorkerRequest in the future becomes L2-writable by default until someone remembers to extend this list. Consider a reflection-based test that asserts every non-whitelisted field of UpdateWorkerRequest appears in the forbidden list (or invert the check to enumerate the allowed fields), so the security boundary degrades to deny-by-default when the request type grows.

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Re-review of the amended head (42335c2): the security findings from the previous review are both addressed. remoteSkills and mcpServers are now closed to default L2 callers (only public-catalog skills remains L2-writable), closing the gateway-key exfiltration path through GenerateMcporterConfig — verified against main: the generator attaches Authorization: Bearer <gatewayKey> to every MCP entry using the URL verbatim, so keeping mcpServers behind the elevated capability (pending #1220) is the right call. The fail-open field-policy concern is resolved by TestL2WorkerUpdateFieldPolicyCoversAllRequestFields, the reflection-based pin that fails if any future UpdateWorkerRequest field lacks an explicit policy decision. The new design doc and the linked design issue #1220 respond to the maintainer's request for a design discussion. Two user-facing doc tables are stale (see inline), and the PR description still describes the old whitelist.

Findings

  • [Warning] docs/usage/resource-management.md:190 — L2 row lists skills, remoteSkills, mcpServers; code now allows only skills (remoteSkills/mcpServers → 400 pending #1220)
  • [Warning] docs/zh-cn/usage/resource-management.md:190 — same stale L2 row in the zh-cn doc

Suggestions

  • Update both usage-doc tables (and the paragraphs under them) to "skills only" with a pointer to the #1220 elevated-capability design; also refresh the PR body's field-whitelist bullet so reviewers don't get conflicting signals.
  • Verified against main@ac22c88: findTeamMember signature matches the call site (first-matching-team semantics, consistent with the read path); TeamMatches/CallerFromContext/CallerKeyForTest all exist; GenerateMcporterConfig bearer injection confirmed. The 404-vs-403 probe resistance and admin/leader regression coverage look consistent with the project-write pattern.
  • Final approval still rests on the maintainer's CHANGES_REQUESTED design review and the #1220 discussion; no APPROVE from this bot at this stage.

Automated review by github-manager-bot

Comment thread docs/usage/resource-management.md Outdated
|------|-------|--------|
| admin / manager | any worker | all fields |
| team leader | workers in their team | all fields |
| L2 human (`permissionLevel: 2`) | workers in `accessibleTeams` only | `skills`, `remoteSkills`, `mcpServers` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This row is stale relative to the tightened implementation: checkHumanWorkerUpdate now rejects remoteSkills and mcpServers for default L2 callers (400, elevated capability pending #1220), so only skills is L2-writable. The design doc (docs/design/l2-worker-scoped-write.md) states this correctly, but this user-facing table still advertises skills, remoteSkills, mcpServers — and the paragraph below the table repeats it. Suggest: skills (public-catalog assignment) only, with a note that remoteSkills/mcpServers require the elevated capability once #1220 lands.

Comment thread docs/zh-cn/usage/resource-management.md Outdated
|------|------|------|
| admin / manager | 任意 Worker | 全部字段 |
| 团队 Leader | 本团队 Worker | 全部字段 |
| L2 人类用户(`permissionLevel: 2`) | 仅 `accessibleTeams` 内的 Worker | `skills`、`remoteSkills`、`mcpServers` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same staleness as the English doc: the L2 row lists skills, remoteSkills, mcpServers, but the code now allows only skills for default L2 (remoteSkills/mcpServers → 400 pending the #1220 elevated-capability design). The surrounding sentence ("L2 人类用户可自主管理…远程注册表技能、MCP 服务器") needs the same correction.

@LUOSENGWA
LUOSENGWA force-pushed the feat/l2-worker-scoped-write branch from 42335c2 to 8393875 Compare September 4, 2026 06:33
@LUOSENGWA LUOSENGWA changed the title feat(controller): allow L2 humans to update worker skills and MCP config within their teams feat(controller): allow L2 humans to update worker skills within their teams Sep 4, 2026

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Re-review of the amended head (8393875): the security lockdown from the previous revision is intact and both stale doc tables flagged in the previous review are fixed. The maintainer's design-level CHANGES_REQUESTED items (separate permission actions instead of generic ActionUpdate, credentialRef, FullAccess-style tiering per #1220) remain open, so the approval decision is left to the maintainer.

Verified:

  • L2 field policy intact — only skills (public-catalog assignment) is L2-writable; remoteSkills and mcpServers are rejected with 400, with the gateway-key exfiltration rationale documented in checkHumanWorkerUpdate. The deny-by-default reflection pin (TestL2WorkerUpdateFieldPolicyCoversAllRequestFields) still guards the policy.
  • Team scope + W8 probe-resistance (cross-team/standalone → 404) unchanged.
  • [Warning → fixed] docs/usage/resource-management.md:190-192 and the zh-cn counterpart now list only skills for L2 and explain why remoteSkills/mcpServers stay gated until #1220.

CI note: integration-tests (controller-cr, SHARD_C_TESTS, qwenpaw, qwenpaw) is failing on this PR's latest run while the same shard passes on #1210 — please investigate whether the failure is PR-specific or flaky.

Local verification: go test ./internal/server/ ./internal/auth/ green on this head.


Automated review by github-manager-bot

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants