fix(import): reject servers that fail config validation before persisting (RC4-IMPORT-001) - #1490
Merged
github-actions[bot] merged 2 commits intoOct 3, 2026
Conversation
…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.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-desktopformat andskip_quarantine=true:POST /api/v1/servers/importreturned HTTP 200 withimported=1, plus a "missing command" warning;command is required for stdio protocol).Fix
config.ValidateServerForLoad(server)runs the boot-pathvalidateDetailedCoreon a single server.configimport.Importis 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 infailedwithinvalid_serverand the reasons, and is never saved. Valid entries in the same payload import as before.working_dirdoesn'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:
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/userentry passes or fails depending on the host.🤖 Generated with Claude Code