What would you require before approving reflect.MethodByName dispatch of admin commands in a Go service?

principalnice to knowfreq 18%

answer

it is an API decision, not a feature
ask first whether it must be reflective
allowlist, not discovery
exported quietly becomes the contract
nothing catches a renamed handler

basics

~20 s

Treat it as opening an API, not adding a feature. Require an allowlist of command names on one handler type, per-command authorization, validation before any call, and a test resolving every registered name so a rename cannot break dispatch silently.

solid answer

~40 s

My first question is whether it needs to be reflective at all: below a few dozen commands a map[string]func(...) registry is strictly better — compile-time references, rename safety, and the linker can still prune. If the team makes the case, I approve it only with an explicit allowlist of command names, dispatch confined to one handler type whose exported methods are exactly the commands, authorization decided per command rather than per endpoint, signature validation before anything reaches Call, and a recover boundary so a mismatch cannot kill the process. I also want a test that resolves every registered name, because without a compile-time reference a rename silently deletes a command. And I make the team write down what they have promised: exporting a method on that type now publishes an operator command.

My first question is whether it needs to be reflective at all: below a few dozen commands a `map[string]func(...)` registry is strictly better — compile-time references, rename safety, and the linker can still prune. If the team makes the case, I approve it only with an explicit allowlist of command names, dispatch confined to one handler type whose exported methods are exactly the commands, authorization decided per command rather than per endpoint, signature validation before anything reaches `Call`, and a `recover` boundary so a mismatch cannot kill the process. I also want a test that resolves every registered name, because without a compile-time reference a rename silently deletes a command. And I make the team write down what they have promised: exporting a method on that type now publishes an operator command.

code

go · 13 lines

func TestEveryCommandResolves(t *testing.T) {
	v := reflect.ValueOf(&Admin{})
	for _, name := range allowedCommands {
		m := v.MethodByName(name)
		if !m.IsValid() {
			t.Errorf("command %s no longer resolves", name)
			continue
		}
		if ft := m.Type(); ft.NumIn() != 1 || ft.NumOut() != 1 {
			t.Errorf("command %s has signature %s", name, ft)
		}
	}
}

go deeper

for a junior

Understand that resolving methods from strings turns the exported method set into a public command list, and that no compiler check stands behind it.

for a middle

Be able to state the concrete costs: no compile-time reference, renames break silently, arguments are checked at run time, and the linker stops pruning methods.

for a senior

Bring the guardrails unprompted — allowlist, dedicated handler type, per-command authorization, pre-flight validation with recover behind it, and a test that resolves every registered name.

for a principal

Own the decision and the smaller alternative. Say who can overrule you, what the team is promising every future contributor, and under what command count the reflective version starts to earn its cost.

Reframe the request

A proposal to dispatch operator commands onto methods by name reads like an implementation detail. It is not. It converts a language-level visibility marker into a public interface: after it lands, exporting a method on a dispatchable type is publishing an operator command. That decision outlives the person who makes it, and everyone who later adds a method to that type will make it again without noticing. That is why it belongs in a review conversation rather than in a commit.

First: does it have to be reflective?

Most admin command buses have between five and thirty commands, added a few times a year. For that shape, a registry of function values is better on every axis a reviewer cares about:

var commands = map[string]func(*Admin, string) error{
    "drain": (*Admin).Drain,
}

The method expressions are ordinary references. Renaming a method breaks the build. The set of commands is greppable and reviewable on one screen. The linker's dead-code elimination keeps working. reflect.MethodByName buys exactly one thing over this: handlers that can be added without touching the registry. Ask what that convenience is worth, and to whom — it is usually worth something to the maintainer and nothing to anyone else.

If it goes ahead, the conditions

An allowlist, not discovery. Commands come from an explicit table mapping the accepted names to what may be invoked. "Whatever exported method matches the string" is the version that fails review, because its contents change without anyone editing the dispatcher.

One dedicated type. Dispatch onto a purpose-built handler struct whose exported methods are the commands and nothing else, embedding no third-party types. This bounds intent even though it cannot bound the linker.

Authorization per command. If authorization is decided at the transport layer, every future command inherits the permissions of the first one. The check keyed by command name, before dispatch, is the only version that stays correct as the table grows.

Validation before the call, recover around it. Operator input arrives as strings. Convert and check each argument against the resolved signature — NumIn, In(i), AssignableTo — and reject mismatches as ordinary errors, because Call reports them as panics. Keep a deferred recover at the dispatch boundary as a net under the validation, never as a substitute for it, and log the resolved command name because the panic message names only types.

A resolution test. A test that iterates the registry, resolves each name against the handler type and asserts the arity and result shape. Without a compile-time reference, a rename silently removes a command, and nothing — not the compiler, not go vet, not an unused-code check, not an IDE rename — will notice.

Written-down consequences. Two, in the package doc: exported means dispatchable on this type, and the reachable reflective lookup stops the linker pruning methods program-wide, so the binary has a floor.

The arguments you will hear

"The allowlist duplicates what MethodByName already does." It does, and that duplication is the product. The table is the reviewed artefact; the reflection is only the mechanism.

"It is internal, behind an admin endpoint." Internal admin endpoints are where an attacker who already has a foothold goes next, and the surface here grows by accident rather than by decision. "Internal" argues for a lighter process, not for an unbounded surface.

"Binary size does not matter to us." Sometimes true. Then say so explicitly and drop that argument — it is the weakest of the three costs and leaning on it undermines the two that matter.

Who decides

The library or service maintainer proposes it and owns the ergonomics. The person who can overrule is whoever owns the security review or the exported API, because the cost lands on them: a surface that changes without a code change to the dispatcher, and a rule about what exporting means that every future contributor must know. A good outcome is not a veto but a smaller version — registry first, reflection only if the command count actually reaches the point where the registry hurts.

## Reframe the request A proposal to dispatch operator commands onto methods by name reads like an implementation detail. It is not. It converts a language-level visibility marker into a public interface: after it lands, exporting a method on a dispatchable type *is* publishing an operator command. That decision outlives the person who makes it, and everyone who later adds a method to that type will make it again without noticing. That is why it belongs in a review conversation rather than in a commit. ## First: does it have to be reflective? Most admin command buses have between five and thirty commands, added a few times a year. For that shape, a registry of function values is better on every axis a reviewer cares about: var commands = map[string]func(*Admin, string) error{ "drain": (*Admin).Drain, } The method expressions are ordinary references. Renaming a method breaks the build. The set of commands is greppable and reviewable on one screen. The linker's dead-code elimination keeps working. `reflect.MethodByName` buys exactly one thing over this: handlers that can be added without touching the registry. Ask what that convenience is worth, and to whom — it is usually worth something to the maintainer and nothing to anyone else. ## If it goes ahead, the conditions **An allowlist, not discovery.** Commands come from an explicit table mapping the accepted names to what may be invoked. "Whatever exported method matches the string" is the version that fails review, because its contents change without anyone editing the dispatcher. **One dedicated type.** Dispatch onto a purpose-built handler struct whose exported methods are the commands and nothing else, embedding no third-party types. This bounds intent even though it cannot bound the linker. **Authorization per command.** If authorization is decided at the transport layer, every future command inherits the permissions of the first one. The check keyed by command name, before dispatch, is the only version that stays correct as the table grows. **Validation before the call, recover around it.** Operator input arrives as strings. Convert and check each argument against the resolved signature — `NumIn`, `In(i)`, `AssignableTo` — and reject mismatches as ordinary errors, because `Call` reports them as panics. Keep a deferred `recover` at the dispatch boundary as a net under the validation, never as a substitute for it, and log the resolved command name because the panic message names only types. **A resolution test.** A test that iterates the registry, resolves each name against the handler type and asserts the arity and result shape. Without a compile-time reference, a rename silently removes a command, and nothing — not the compiler, not `go vet`, not an unused-code check, not an IDE rename — will notice. **Written-down consequences.** Two, in the package doc: exported means dispatchable on this type, and the reachable reflective lookup stops the linker pruning methods program-wide, so the binary has a floor. ## The arguments you will hear *"The allowlist duplicates what MethodByName already does."* It does, and that duplication is the product. The table is the reviewed artefact; the reflection is only the mechanism. *"It is internal, behind an admin endpoint."* Internal admin endpoints are where an attacker who already has a foothold goes next, and the surface here grows by accident rather than by decision. "Internal" argues for a lighter process, not for an unbounded surface. *"Binary size does not matter to us."* Sometimes true. Then say so explicitly and drop that argument — it is the weakest of the three costs and leaning on it undermines the two that matter. ## Who decides The library or service maintainer proposes it and owns the ergonomics. The person who can overrule is whoever owns the security review or the exported API, because the cost lands on them: a surface that changes without a code change to the dispatcher, and a rule about what exporting means that every future contributor must know. A good outcome is not a veto but a smaller version — registry first, reflection only if the command count actually reaches the point where the registry hurts.

The team argues an allowlist just duplicates what MethodByName already does. How do you answer?
The duplication is the point. The table is the artefact a reviewer approves and a newcomer reads; the reflection is only how the call is made. Without it, the set of operator commands changes whenever someone exports a method, with no diff to the dispatcher and nothing for a reviewer to catch.
What single test would you insist on, and why that one?
One that iterates the registered names, resolves each against the handler type and asserts the arity and result shape. Because there is no compile-time reference, a rename or a signature change deletes a command silently — the compiler, vet, unused-code detection and IDE renames all miss it. That test is the only thing standing in for the reference the code no longer has.
When would you accept the reflective version over a registry?
When the command count is genuinely large and grows from several teams, when the handlers are uniform enough that a table adds only noise, and when nobody is paying for binary size. Even then I would keep the allowlist and the resolution test; what I would drop is the argument that the registry is cheaper to maintain.
How do you frame the binary-size cost without overstating it?
As the third argument, not the first. A reachable reflective lookup stops the linker pruning methods program-wide, so the binary gains a floor and deleting dead code stops helping. On a service where the artefact ships widely that is real; where it does not, say so and rest the case on the surface and the lost rename safety instead.

saying these in an interview costs you the question

Treats it as an implementation detail rather than an API decision
Accepts open discovery of exported methods with no allowlist
Puts authorization at the endpoint instead of per command
Relies on recover with no signature validation
Has no test proving each registered name still resolves
Argues internal admin endpoints need no surface review

What would you require before approving reflect.MethodByName dispatch of admin commands in a Go service?

§ answer

basics

solid answer

code

§ go deeper

▾level notes4 levels

▾deep dive~3 min

Reframe the request

First: does it have to be reflective?

If it goes ahead, the conditions

The arguments you will hear

Who decides

▾common follow-ups4 questions

▾red flags6 flags

Reframe the request

First: does it have to be reflective?

If it goes ahead, the conditions

The arguments you will hear

Who decides

answer

go deeper