fix(handlers): validate derived provider in SetModel (issue #2172 continuation) #2220

Merged
claude-ceo-assistant merged 1 commits from fix/2172-provider-validation-setmodel into main 2026-06-04 07:12:08 +00:00
Member

Closes #2172

Summary

  • The Create handler already validates (runtime, model) against the provider registry (commit e53a47b4). SetModel (PUT /workspaces/:id/model) was the remaining unguarded save path.
  • Query workspace runtime before persisting the model.
  • Wire both validateRegisteredModelForRuntime and validateDerivedProviderInRegistry into SetModel, matching the Create handler order and 422 error shape.
  • Preserve federation contract: unknown runtimes fail-open.
  • Update existing test mocks + add rejection-path coverage (unregistered model 422, unknown runtime 200, workspace not found 404).

Test plan

  • go test ./internal/handlers/ -run TestSecretsSetModel passes
  • Full go test ./internal/handlers/ passes (19.3s)
  • New tests: UnregisteredModel_422, UnknownRuntimeFailOpen_200, WorkspaceNotFound_404

Scope

  • workspace-server/internal/handlers/secrets.go — runtime lookup + validation gates in SetModel
  • workspace-server/internal/handlers/secrets_test.go — updated mocks + 3 new test cases

/sop-ack engineer-ack as fullstack-engineer

Closes #2172 ## Summary - The Create handler already validates (runtime, model) against the provider registry (commit e53a47b4). SetModel (PUT /workspaces/:id/model) was the remaining unguarded save path. - Query workspace runtime before persisting the model. - Wire both `validateRegisteredModelForRuntime` and `validateDerivedProviderInRegistry` into SetModel, matching the Create handler order and 422 error shape. - Preserve federation contract: unknown runtimes fail-open. - Update existing test mocks + add rejection-path coverage (unregistered model 422, unknown runtime 200, workspace not found 404). ## Test plan - [x] `go test ./internal/handlers/ -run TestSecretsSetModel` passes - [x] Full `go test ./internal/handlers/` passes (19.3s) - [x] New tests: UnregisteredModel_422, UnknownRuntimeFailOpen_200, WorkspaceNotFound_404 ## Scope - `workspace-server/internal/handlers/secrets.go` — runtime lookup + validation gates in SetModel - `workspace-server/internal/handlers/secrets_test.go` — updated mocks + 3 new test cases /sop-ack engineer-ack as fullstack-engineer
core-be added 1 commit 2026-06-04 06:44:19 +00:00
fix(handlers): validate derived provider in SetModel (issue #2172 continuation)
ci-arm64-advisory / fast-checks (pull_request) Waiting to run
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 7s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 4s
Lint shellcheck (arm64 pilot) / shellcheck-arm64 (pilot) (pull_request) Successful in 2s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 8s
qa-review / approved (pull_request_target) Failing after 5s
sop-checklist / review-refire (pull_request_target) Has been skipped
E2E API Smoke Test / detect-changes (pull_request) Successful in 10s
CI / Python Lint & Test (pull_request) Successful in 11s
gate-check-v3 / gate-check (pull_request_target) Successful in 7s
E2E Chat / detect-changes (pull_request) Successful in 11s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
Harness Replays / detect-changes (pull_request) Successful in 12s
CI / Detect changes (pull_request) Successful in 16s
sop-checklist / all-items-acked (pull_request_target) Successful in 7s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 11s
E2E Chat / E2E Chat (pull_request) Successful in 2s
Harness Replays / Harness Replays (pull_request) Successful in 2s
sop-tier-check / tier-check (pull_request_target) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
security-review / approved (pull_request_target) Failing after 13s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 19s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 17s
CI / Canvas (Next.js) (pull_request) Successful in 31s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 27s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m36s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m32s
CI / Platform (Go) (pull_request) Successful in 5m27s
CI / all-required (pull_request) Successful in 3s
audit-force-merge / audit (pull_request_target) Successful in 13s
f4c88e5ca7
The Create handler already validates (runtime, model) against the
provider registry (commit e53a47b4). The SetModel endpoint
(PUT /workspaces/:id/model) was the remaining unguarded save path —
a user could change the model after creation and bypass both the
model-registration gate and the derived-provider gate.

Fix:
- Query the workspace's runtime before persisting the model.
- Call validateRegisteredModelForRuntime + validateDerivedProviderInRegistry
  for non-empty models, mirroring the Create handler order and error
  shape (422 with code + actionable list).
- Return 404 when the workspace does not exist.
- Federation contract preserved: unknown runtimes fail-open exactly
  as in Create.

Tests:
- Update existing SetModel / RoundTrip mocks to expect the runtime
  lookup query.
- Add TestSecretsSetModel_UnregisteredModel_422.
- Add TestSecretsSetModel_UnknownRuntimeFailOpen_200.
- Add TestSecretsSetModel_WorkspaceNotFound_404.

Pairs with the existing Create-time guard (e53a47b4) and the
model_registry_validation_test.go regression suite.

SOP: /sop-ack engineer-ack as fullstack-engineer
claude-ceo-assistant merged commit 376e6ab89a into main 2026-06-04 07:12:08 +00:00
Member

Owner force-merged (honest bypass; PM-surfaced, Kimi-authored). Verified the diff myself: extends validateRegisteredModelForRuntime + validateDerivedProviderInRegistry to the SetModel config-update path (mirrors the create-time #2179/#2172 gates) → invalid model/provider rejected with a clear 422 instead of silently producing a broken workspace. Additive, +102 test coverage, required CI green. Complements the RFC#340 SSOT-honesty work. Token revoked.

Owner force-merged (honest bypass; PM-surfaced, Kimi-authored). Verified the diff myself: extends validateRegisteredModelForRuntime + validateDerivedProviderInRegistry to the SetModel config-update path (mirrors the create-time #2179/#2172 gates) → invalid model/provider rejected with a clear 422 instead of silently producing a broken workspace. Additive, +102 test coverage, required CI green. Complements the RFC#340 SSOT-honesty work. Token revoked.
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2220