Skip to content

fix(cli): require explicit source for unknown-server fallback#7

Merged
lydakis merged 2 commits intomainfrom
codex/fix-vulnerability-for-unknown-server-names
Mar 12, 2026
Merged

fix(cli): require explicit source for unknown-server fallback#7
lydakis merged 2 commits intomainfrom
codex/fix-vulnerability-for-unknown-server-names

Conversation

@lydakis
Copy link
Owner

@lydakis lydakis commented Mar 12, 2026

Motivation

  • Prevent implicit ephemeral resolution for bare/typo server names that allowed local files in the working directory to be treated as manifests and executed.

Description

  • Add a guard in sendServerRequestWithEphemeralFallback to only attempt ephemeral source resolution when the requested server string looksLikeExplicitEphemeralSource (path/URL/install-link/json/toml/config-like) in internal/cli/root.go.
  • Tighten regression coverage in internal/cli/root_test.go to assert resolveSourceFn and checkPrereqsFn are not invoked for implicit unknown-server fallbacks (typo names).
  • Preserve existing retry behavior for explicit ephemeral sources (absolute/relative paths, URLs, files with .json/.toml, or sources containing :// or ?config=).

Testing

  • Ran go test ./internal/cli and the CLI tests passed.
  • Ran go vet ./... and go build ./... which both succeeded.
  • Ran go test ./... which failed in internal/config due to unrelated, environment-dependent tests (TestLoadCodexConfigFileAddsCodexAppsServerFromAuthFile and TestLoadCodexConfigFileCodexAppsUsesConnectorsTokenEnv); the failures are preexisting and unrelated to this change.

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e94e50ea46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +759 to +760
if !looksLikeExplicitEphemeralSource(effectiveReq.Server) {
return resp, nil

Choose a reason for hiding this comment

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

P2 Badge Treat bare relative file paths as explicit sources

This new guard skips fallback unless looksLikeExplicitEphemeralSource is true, but that helper does not recognize valid relative file paths without / or a .json/.toml suffix (for example mcpx manifest ping {} when ./manifest is a JSON/TOML manifest). Before this commit those sources resolved via bootstrap.Resolve (which reads arbitrary file paths), so this change regresses legitimate ephemeral-source invocations into unknown server errors.

Useful? React with 👍 / 👎.

@lydakis lydakis merged commit 20992af into main Mar 12, 2026
7 checks passed
@lydakis lydakis deleted the codex/fix-vulnerability-for-unknown-server-names branch March 12, 2026 04:47
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02c69f4480

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +864 to +868
info, err := statSourceFn(filepath.Clean(source))
if err != nil {
return false
}
return info.Mode().IsRegular()

Choose a reason for hiding this comment

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

P2 Badge Keep typo handling from probing bare local files

The new os.Stat fallback makes any existing regular file look like an explicit source, so an invocation like mcpx ghub ping {} now enters ephemeral resolution whenever ./ghub exists, even if the user meant a server name and that file is unrelated. In this case parse/validation failures are surfaced as resolving source ... errors instead of the previous unknown server response, which regresses typo UX and reintroduces local-file influence for bare names in normal project directories.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant