Skip to content

Gate gRPC control plane startup behind API flag#6

Merged
homerquan merged 1 commit into
mainfrom
codex/fix-unauthenticated-grpc-control-plane
May 18, 2026
Merged

Gate gRPC control plane startup behind API flag#6
homerquan merged 1 commit into
mainfrom
codex/fix-unauthenticated-grpc-control-plane

Conversation

@homerquan

Copy link
Copy Markdown
Collaborator

Motivation

  • The application previously started the gRPC control-plane supervisor unconditionally, exposing job/cluster/observability RPCs on the network without honoring the existing API enablement setting. This change restricts that network surface behind the runtime API flag.

Description

  • Replace the unconditional gRPC supervisor child spec with a grpc_child_specs/0 helper that only returns the GRPC.Server.Supervisor spec when MN_API_ENABLED is true (via Config.boolean("MN_API_ENABLED", :api_enabled)).
  • Preserve existing behavior for enabled mode by continuing to honor MN_GRPC_PORT and MN_CORE_HOST and by keeping the loopback default binding when MN_CORE_HOST is empty or localhost.
  • Add tests/unit/application_test.exs with unit coverage asserting the gRPC child spec is omitted when the API is disabled and produced (with loopback binding) when enabled.
  • Update README.md to document that MN_API_ENABLED controls the gRPC listener and that the default MN_CORE_HOST is loopback-only.

Testing

  • Ran formatting checks with mix format --check-formatted which passed locally for the modified files.
  • Performed Elixir syntax/AST checks via elixir -e 'Code.string_to_quoted!(File.read!(...))' which validated the modified modules as syntactically correct.
  • Executed a local evaluation of MirrorNeuron.Application.grpc_child_specs() under both MN_API_ENABLED=false and MN_API_ENABLED=true to confirm the expected child-spec behavior (observed expected results).
  • Attempted to run mix test and mix test tests/unit/application_test.exs, but fetching Hex/dependencies failed due to an external HTTP/SSL error (mix local.hex --force / mix deps.get failed with an HTTP 403 SSL tunnel error), so automated test execution was blocked and full test-suite results are unavailable.

Codex Task

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

Copy link
Copy Markdown

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: f062d4b19e

ℹ️ 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".


@doc false
def grpc_child_specs do
if Config.boolean("MN_API_ENABLED", :api_enabled) do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip gRPC port validation when API is disabled

When MN_API_ENABLED=false, this gate suppresses the gRPC child, but start/2 still calls Config.validate!/0 first, and that unconditionally validates MN_GRPC_PORT in lib/mirror_neuron/config.ex. In a deployment that disables the API listener and has an intentionally unset/non-port placeholder for MN_GRPC_PORT, the application still fails during startup even though the port will never be used, so the new disable flag does not fully prevent gRPC startup requirements from applying.

Useful? React with 👍 / 👎.

@homerquan homerquan merged commit a3306ef into main May 18, 2026
1 check passed
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