Skip to content

[fix][evaluation] enforce authorization on eval-target debug/async-debug, template update, and evaluator delete/draft-update#585

Merged
topjames666 merged 4 commits into
mainfrom
fix/evaluation-authz-7308607569
Jul 20, 2026
Merged

[fix][evaluation] enforce authorization on eval-target debug/async-debug, template update, and evaluator delete/draft-update#585
topjames666 merged 4 commits into
mainfrom
fix/evaluation-authz-7308607569

Conversation

@topjames666

@topjames666 topjames666 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What

Fix authorization gaps in the evaluation module's application layer:

  1. DebugEvalTarget / AsyncDebugEvalTarget — the authorization call was
    commented out in both the sync and async eval-target debug handlers, so any
    authenticated caller could trigger a debug run against an arbitrary
    workspace_id. Restored the space-level Authorization check
    (ActionDebugEvalTarget / AuthEntityType_Space) on both. (The frontend
    "debug target" feature calls both endpoints depending on sync/async mode, so
    both must be fixed.)

  2. UpdateExperimentTemplate — the write path only checked space-level
    read/list permission, allowing a low-privilege member to modify templates
    owned by others. Switched to an object-level edit check
    (AuthorizationWithoutSPI with the template's owner), consistent with
    UpdateExperiment.

  3. DeleteEvaluator — authorization relied on the result of an unfiltered
    batch lookup; an empty result silently skipped the auth check and fell
    through to a global-id soft delete. Reworked to use the space-scoped
    GetEvaluator, return EvaluatorNotExistCode when not found, and authorize
    against the resolved object — aligned with UpdateEvaluator.

  4. UpdateEvaluatorDraft — the CustomRPC/Agent write gate keyed off the
    client-supplied evaluator_type, while persistence used the server object's
    real type (type confusion): a caller could declare a non-CustomRPC type to
    skip the gate yet still write CustomRPC content. The gate now keys off the
    server-side type, plus a consistency check that rejects a request whose
    declared type does not match the stored object.

Why

Each is an authorization boundary defect: missing (1), downgraded to a weaker
permission (2), auth object vs. write object mismatch (3), or auth decision
driven by client-controllable input (4).

Tests

Added/updated unit tests covering the unauthorized/mismatched paths for all
handlers (permission-denied, not-found rejection, cross-space rejection,
type-mismatch rejection). All evaluation application tests pass:
cd backend && go test -gcflags="all=-N -l" ./modules/evaluation/application/...

topjames666 and others added 3 commits July 20, 2026 11:45
DebugEvalTarget 入口的 space 级 debug 鉴权此前被整段注释,任何登录用户可跨 workspace 触发调试执行并写入评测记录。恢复 e.auth.Authorization 校验,补充无权限被拒单测。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…emplate

写接口此前只做空间级 list/read 权限校验,同空间低权限用户可越权修改他人模板。改为先取模板拿 owner,再用 AuthorizationWithoutSPI 做对象级 edit 校验(对齐 UpdateExperiment),补充无 edit 权限被拒单测。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ype confusion

DeleteEvaluator 改用带空间校验的 GetEvaluator(跨空间/查不到返回 NotExist 并拒删),消除空结果跳过鉴权的隐患;UpdateEvaluatorDraft 的 CustomRPC/Agent 写门禁改按服务端真实类型触发,并增加请求类型与服务端类型一致性校验,堵住类型混淆写入。补充相应单测。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
VinCinx
VinCinx previously approved these changes Jul 20, 2026
xueyizheng
xueyizheng previously approved these changes Jul 20, 2026
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #585      +/-   ##
==========================================
+ Coverage   77.44%   77.47%   +0.02%     
==========================================
  Files         678      679       +1     
  Lines       80813    80845      +32     
==========================================
+ Hits        62589    62631      +42     
+ Misses      14491    14484       -7     
+ Partials     3733     3730       -3     
Flag Coverage Δ
unittests 77.47% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../modules/evaluation/application/eval_target_app.go 93.38% <100.00%> (+0.18%) ⬆️
...nd/modules/evaluation/application/evaluator_app.go 90.43% <100.00%> (+0.11%) ⬆️
...d/modules/evaluation/application/experiment_app.go 83.22% <100.00%> (+0.14%) ⬆️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b9b2a8...1e95382. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

AsyncDebugEvalTarget 与 DebugEvalTarget 同源:入口鉴权此前被整段注释,异步调试路径可未授权跨 workspace 触发。恢复 space 级 debug 鉴权,补无权限被拒单测。前端对象调试功能同时使用同步/异步两条路径,需一并修复才能真正闭合。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@topjames666
topjames666 dismissed stale reviews from xueyizheng and VinCinx via 1e95382 July 20, 2026 08:06
@topjames666 topjames666 changed the title [fix][evaluation] enforce authorization on eval-target debug, template update, and evaluator delete/draft-update [fix][evaluation] enforce authorization on eval-target debug/async-debug, template update, and evaluator delete/draft-update Jul 20, 2026
@xueyizheng
xueyizheng self-requested a review July 20, 2026 08:29
@VinCinx
VinCinx self-requested a review July 20, 2026 08:33
@topjames666
topjames666 merged commit bf89c83 into main Jul 20, 2026
11 checks passed
@topjames666
topjames666 deleted the fix/evaluation-authz-7308607569 branch July 20, 2026 08:34
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