Skip to content

fix(import): reject servers that fail config validation before persisting (RC4-IMPORT-001) - #1490

Merged
github-actions[bot] merged 2 commits into
mainfrom
claude/fix-import-invalid-server-persist
Oct 3, 2026
Merged

github-actions[bot] merged 2 commits into
mainfrom
claude/fix-import-invalid-server-persist

Conversation

@Dumbris

@Dumbris Dumbris commented Oct 3, 2026

Copy link
Copy Markdown
Member

Pull Request

Description

Fixes RC4-IMPORT-001, from the codex QA of v0.70.0-rc.4. The bug also exists in rc.2.

A malformed import was accepted and persisted, and then bricked the next startup. The trigger was a URL-only entry imported with a forced claude-desktop format and skip_quarantine=true:

  • POST /api/v1/servers/import returned HTTP 200 with imported=1, plus a "missing command" warning;
  • it persisted a stdio server with no command;
  • the next core start exited with code 4 (command is required for stdio protocol).

Fix

  • New config.ValidateServerForLoad(server) runs the boot-path validateDetailedCore on a single server.
  • configimport.Import is the single choke point for REST import and CLI import, in daemon and direct-file mode. It now validates each mapped server before offering it for persistence. A server that fails is reported in failed with invalid_server and the reasons, and is never saved. Valid entries in the same payload import as before.
  • The REST import applies the caller's rename map after that validation, so renamed servers are re-validated before anything is persisted.
  • Side effect, intended: an entry whose working_dir doesn't exist on this machine is now rejected too, because startup would refuse it.

Testing

  • I have tested these changes locally

  • I have added/updated tests that prove my fix is effective or my feature works

  • All existing tests pass

  • New tests:

    • configimport: the malformed entry is rejected with its reason, and a valid entry in the same payload still imports;
    • httpapi: TestRunImport_RenameToInvalidNameIsRejected.
  • Suites: go test ./internal/configimport/ ./internal/httpapi/ passes.

  • Review: zcode found two lows. The rename re-validation is fixed here. The codex-fixture assertion is now environment-tolerant, because its cwd=/home/user entry passes or fails depending on the host.

🤖 Generated with Claude Code

…ting

POST /api/v1/servers/import with a URL-only entry forced through the
claude-desktop format mapped to a commandless stdio server, which was
persisted (notably with skip_quarantine=true) and made the next core
startup exit with code 4 (command is required for stdio protocol).

configimport.Import, the single choke point behind REST import and the CLI
upstream import (daemon and direct config-file mode), now runs the same
per-server checks config.Load applies (new config.ValidateServerForLoad)
and reports failing entries as Failed (invalid_server, with the reason)
instead of offering them for persistence. Valid entries in the same
payload import as before.
configimport validates each server under its pre-rename name; the REST
import then applies the caller's rename map. Re-run the boot-path
validation after renaming so a rename to a name the next startup refuses
(e.g. containing ':') fails the entry instead of persisting it. From the
zcode review of RC4-IMPORT-001.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved (Model B): Paperclip review verdicts = ACCEPT and qa-gate green at this head SHA. Arming auto-merge; GitHub merges when all required checks pass.

@github-actions
github-actions Bot enabled auto-merge (squash) October 3, 2026 18:00
@github-actions
github-actions Bot merged commit a010eca into main Oct 3, 2026
43 checks passed
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 74.28571% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/config/config.go 0.00% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

2 participants