DEV Community

Cover image for Not Found is the wrong answer to a permission problem
Nasrul Hazim
Nasrul Hazim

Posted on

Not Found is the wrong answer to a permission problem

I spent today putting an MCP surface on a multi-tenant Laravel control plane — the kind of app where a wrong answer doesn't just render badly, it tears down infrastructure. Sixty-odd tools across four servers, and honestly the tools were the easy part. The hard part was the base class every tool sits on, and most of what went into it came from getting it wrong first.

Here's the thing about exposing an app to an agent: every assumption your web UI quietly relies on stops holding. There's no session. There's no human reading the screen and going "hmm, that's odd." An agent takes your response literally, and then acts on it. So the design question stops being "what can this tool do" and becomes "what does this tool say when it refuses, and can the thing reading it tell the difference between refusals?"

Four guards, in order, each one a bug I shipped once

The base tool runs four checks before a tool's own code executes:

  1. Is there an authenticated user at all?
  2. Does their role grant this server?
  3. Does this token carry the required ability?
  4. Does their role grant this tool's domain permission?

Then it resolves a tenant. That's the shape:

abstract class McpTool extends Tool
{
    use ScopesToTenant;

    /** Which server this tool belongs to — `mcp.access.{server}`. */
    abstract protected function serverPermission(): string;

    /** The tool's own work. Guards have run; actor() and organization() are non-null. */
    abstract protected function run(Request $request): Response;

    protected function permission(): ?string
    {
        return null;
    }

    /** Read-only tools inherit this. Writers must say so. Destroyers must say `mcp:destroy`. */
    protected function ability(): string
    {
        return 'mcp:read';
    }

    final public function handle(Request $request): Response
    {
        try {
            $this->boot($request);

            return $this->run($request);
        } catch (McpException $e) {
            return $this->error($e->errorCode, $e->getMessage());
        } catch (ValidationException $e) {
            return $this->error(
                McpErrorCode::InvalidArgument,
                implode(' ', $e->validator->errors()->all()),
            );
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

handle() is final and run() is abstract on purpose. If a tool author can override the entry point, one of sixty tools eventually will, and it'll be the one that destroys things.

Note that the guards throw rather than return a Response. That was a deliberate switch partway through. Guards compose — server, ability, permission, tenant, then the record itself — and returning a refusal from each puts five if ($denied) return ... blocks at the head of every tool. That's precisely the shape where the sixth call site forgets one. Throw a typed exception, catch it once in handle(), and forgetting becomes impossible rather than merely unlikely.

The one I reverted: hiding tools you're not allowed to call

MCP has shouldRegister() — return false and the tool doesn't appear in the caller's tool list at all. Tempting. My first pass filtered on every permission a tool needed, so you only saw what you could run.

It's wrong, and it took a confused colleague to see why.

A tool that isn't registered answers "Tool not found." For someone pointed at a server their role doesn't grant, that's honest — the whole surface genuinely isn't theirs, and an empty tool list says so. But for someone who's on the right server and merely lacks one domain permission, "not found" is a lie with a cost attached. It sends them hunting for a typo in the tool name. What they needed to hear was:

This action requires the deployments.view.operations permission.

That's a sentence they can take to an administrator. "Not found" isn't.

So shouldRegister() filters on the server permission only, and per-tool permissions are enforced in boot(), where the refusal can name what's missing:

public function shouldRegister(Request $request): bool
{
    $user = $request->user();

    return $user instanceof User && $this->allows($user, $this->serverPermission());
}

protected function boot(Request $request): void
{
    // ...
    if (! $this->allows($user, $this->serverPermission())) {
        throw McpException::serverAccessDenied($this->serverPermission());
    }
    // ...
}
Enter fullscreen mode Exit fullscreen mode

Notice the server permission is checked twice — once to hide, once to refuse. shouldRegister() is never the lock. A tool list can be stale, cached, or replayed by a client that connected under a different role; if the only thing keeping a tool out of reach is that you didn't list it, the list becomes the security boundary. It isn't one. It's a UI affordance.

Generalise it: visibility is a courtesy, authorisation is a check. Any time you're tempted to hide something instead of denying it, ask what happens when the caller guesses the name.

The guard mismatch that denies everything, superadmin included

This one cost me a good half hour of staring at a role that plainly had the permission.

MCP requests authenticate on sanctum. Roles and permissions are registered on web. A bare $user->can('...') resolves against the active guard — where Spatie has no such permission registered — so every check returns false. Every single one. Including for a superadmin, which is the detail that makes it so misleading: it looks exactly like a broken role assignment, so you go and debug the role.

protected function allows(User $user, string $permission): bool
{
    if ($user->hasRole('superadmin')) {
        return true;
    }

    try {
        return $user->hasPermissionTo($permission, 'web');
    } catch (PermissionDoesNotExist) {
        return false;
    }
}
Enter fullscreen mode Exit fullscreen mode

Two things worth keeping:

Pin the guard explicitly. Not "it works locally because the session guard happened to be active" — name it.

Fail closed on an unknown permission. A typo'd permission string throws PermissionDoesNotExist, and letting that bubble up turns a policy question into a 500 from the authorisation boundary. Catching it and returning false means a typo denies rather than explodes. Which side you'd rather be wrong on isn't a close call.

The general rule: any surface authenticating on a non-default guard has to pin the guard it checks permissions against. Same trap bites API-token surfaces on Passport.

Permissions and abilities are not the same thing

A permission says what the human may do. A token ability says what this credential may do. They're independent, and collapsing them is how a read-only CI token ends up able to tear down production because the person who minted it happens to be an admin.

Every tool declares its tier — mcp:read by default, mcp:write if it mutates, mcp:destroy if it's irreversible — and the check is:

protected function assertAbility(Request $request): void
{
    $token = $this->accessToken();

    if ($token === null) {
        return; // no token, no ability list to narrow against
    }

    if (! $token->can($this->ability())) {
        throw McpException::abilityDenied($this->ability());
    }
}
Enter fullscreen mode Exit fullscreen mode

Tokens minted before abilities existed carry Sanctum's * wildcard and pass everything. That's Sanctum's own semantics and I left it alone deliberately — retroactively narrowing a wildcard would break every already-connected client silently, at some unpredictable later moment. Migrate credentials forward on purpose; don't change what an existing one means underneath its holder.

No session means your tenant resolution is guessing

This is the one I'd flag hardest to anyone doing the same thing.

The app has a current_organization() helper. Reads the session, and when there's no session, falls back to "the first organisation you own." Perfectly reasonable for the web app, where the fallback fires roughly never.

Under a token there is never a session. So the fallback isn't a fallback anymore, it's the primary path — and a user who belongs to three organisations silently addresses whichever one sorts first. The agent has no idea. It asks for "the deployments," gets a list, and reports it as the answer.

Two rules came out of this:

Every tool takes an optional explicit organization. Wins over any default.

Every response echoes the organisation it resolved. Not as a debug field — as part of the contract:

protected function respond(array $payload): Response
{
    if ($this->organization !== null) {
        $payload['organization'] = [
            'uuid' => $this->organization->uuid,
            'name' => $this->organization->name,
        ];
        $payload['cross_tenant'] = $this->crossTenant;
    }

    return $this->json($payload);
}
Enter fullscreen mode Exit fullscreen mode

An ambiguous answer that states its own scope stops being ambiguous. That's the cheapest fix available for a whole class of "it returned the wrong data" bugs, and it applies well beyond MCP — any API where scope is inferred rather than supplied should be telling you what it inferred.

The related decision: reaching another tenant is a named capability, mcp.access.cross-tenant, deliberately not inherited from the general organizations.* permissions. It's a capability flag, not a scope — per-resource access still runs through policies. And a cross-tenant read is written to the target organisation's audit trail:

// An ops user reading a customer's tenant is legitimate — and must still
// leave a trace. Written against the organisation, so it surfaces on that
// tenant's own trail rather than only in an internal log.
Audit::create([
    'user_id' => $this->actor()->id,
    'event' => 'mcp_cross_tenant_access',
    'auditable_type' => $organization->getMorphClass(),
    'auditable_id' => $organization->id,
    'new_values' => ['tool' => $this->name(), 'server' => $this->serverPermission()],
    'tags' => 'mcp',
]);
Enter fullscreen mode Exit fullscreen mode

Support reading a customer's data is a normal, legitimate thing. The customer being able to see that it happened is what makes it normal.

One more, and it's structural: this app has no global scopes. Every query scopes itself or it leaks. So tools never reach a record with a bare where('uuid', ...) — they go through finders on a ScopesToTenant trait. Ownership enforced at five call sites is ownership forgotten at the sixth.

Give the agent a vocabulary, not just prose

An agent that only ever receives sentences can't distinguish "you may not do this" from "this doesn't exist" from "your token is too narrow" — and those call for completely different next moves. Retrying the first two is pointless noise. Retrying the third after minting a wider token is exactly right.

So there's an enum:

enum McpErrorCode: string implements Contract
{
    use InteractsWithEnum;

    case NotAuthenticated = 'not_authenticated';
    case ServerAccessDenied = 'server_access_denied';
    case PermissionDenied = 'permission_denied';
    case AbilityDenied = 'ability_denied';
    case CrossTenantDenied = 'cross_tenant_denied';
    case NotFound = 'not_found';
    case InvalidArgument = 'invalid_argument';
    case ConfirmationMismatch = 'confirmation_mismatch';
    case CapabilitySimulated = 'capability_simulated';
    case Conflict = 'conflict';

    /**
     * Whether the caller could succeed by supplying different arguments.
     * A retryable error is worth trying again; an authorisation failure is
     * not, and an agent that retries one just generates noise against a
     * boundary that will refuse it every time.
     */
    public function isRetryable(): bool
    {
        return $this === self::InvalidArgument
            || $this === self::ConfirmationMismatch
            || $this === self::NotFound;
    }
}
Enter fullscreen mode Exit fullscreen mode

The code travels alongside the human message, never instead of it — a client with no code support still gets something readable:

protected function error(McpErrorCode $code, string $message): Response
{
    return Response::error(json_encode([
        'error' => [
            'code' => $code->value,
            'message' => $message,
            'retryable' => $code->isRetryable(),
        ],
    ]));
}
Enter fullscreen mode Exit fullscreen mode

isRetryable() living on the enum rather than in each tool is the usual argument for enums-with-behaviour: the classification is a property of the error kind, not of the caller. Same reason label() and color() belong there.

Destructive tools: make the argument come from something the caller read

Three tools here can't be undone. An agent asked to "clean up the old deployments" will cheerfully call destroy on whatever it matched, and whatever it matched is doing an enormous amount of load-bearing work in that sentence.

The lock is a typed confirmation — the resource's own name, retyped exactly:

protected function confirmOrFail(Request $request, Model $subject, string $expected): void
{
    $supplied = $request->get('confirm_name');

    if (! is_string($supplied) || $supplied !== $expected) {
        $this->auditDestructive($subject, 'mcp_destructive_refused', [
            'expected' => $expected,
            'supplied' => is_string($supplied) ? $supplied : null,
        ]);

        throw McpException::confirmationMismatch($expected);
    }
}
Enter fullscreen mode Exit fullscreen mode

The point isn't friction for its own sake. It's that the destructive argument now has to come from something the caller actually read — a prior get-deployment call — rather than from a pattern it inferred from the request. Same reasoning as making a human type a count instead of clicking a browser confirm.

And note the refusal is audited too. A refused destruction is evidence: it says something tried. Logging only successes gives you a record that's blind to exactly the events you'd most want to know about.

Combined with a mcp:destroy token ability, a state check on the model's lifecycle enum, and an #[IsDestructive] annotation so a well-behaved client can prompt its human, that's four independent things that have to line up. For an irreversible action against real infrastructure, four feels about right.

Say when the work is only queued

Last one, and it's my favourite because it's a bug this repo already shipped in the UI.

A lifecycle tool dispatches a job and returns. If the response says "deployment stopping," the agent reports it as done. Meanwhile the job is sitting unclaimed, because the work goes onto the database connection and Horizon's supervisors watch redis.

protected function queuedNote(string $what): array
{
    return [
        'queued' => true,
        'connection' => self::QUEUE_CONNECTION,
        'worker_required' => true,
        'message' => __(':what queued on the `database` connection — this needs a worker '
            .'(`php artisan queue:work database`). Horizon does not claim these jobs.', ['what' => $what]),
        'worker_appears_alive' => $this->workerAppearsAlive(),
    ];
}

/**
 * A cheap liveness hint, not a guarantee: jobs older than two minutes mean
 * nothing is claiming them. Null means the question could not be asked,
 * which is deliberately distinct from "yes".
 */
protected function workerAppearsAlive(): ?bool
{
    try {
        return DB::table('jobs')
            ->where('created_at', '<=', now()->subMinutes(2)->getTimestamp())
            ->count() === 0;
    } catch (Throwable) {
        return null;
    }
}
Enter fullscreen mode Exit fullscreen mode

The ?bool matters. null means "couldn't check," and that is not the same as true. Any time a health signal can fail to be read, the un-readable case needs its own value — collapsing it into the optimistic one is how monitoring lies to you.

Server instructions are where the reading rules live

Each server ships #[Instructions(...)], and I ended up using it for something I didn't expect: not "here are the tools" but "here is how to read an answer without drawing the wrong conclusion." Things like:

  • Unscanned is not clean. An empty vulnerability list may mean nothing was found, or nothing was looked at.
  • An unknown severity stays unknown. Mapping it down to "low" launders an unknown into a reassurance.
  • No recorded activity is not "inactive." The audit trail records actions; someone who signs in and only reads leaves no trace.
  • A control with no evidence is unevidenced, not failing. Different states, different responses.
  • An overdue clock reports negative hours, not clamped to zero — overdue is the most important thing it can say.

That's institutional knowledge that normally lives in a senior engineer's head and gets passed on in code review. Writing it into the server instructions is the first time I've had somewhere obvious to put it. Worth doing even if you never connect an agent, frankly.

What I'd take to the next one

  • Refusals are an API. Design them as carefully as the success payloads. A wrong refusal costs more than a missing feature, because the caller acts on it.
  • Hide for convenience, deny for security. Check the same thing in both places.
  • Non-default guard? Pin it. Explicitly, everywhere.
  • A default that only fires without a session becomes the primary path under tokens. Go and read every fallback in your app with that in mind.
  • Echo the scope you inferred. Cheapest ambiguity fix there is.
  • Anything that can't answer a health question needs a third value. Not the optimistic one.

Next up is exercising the whole thing from a real client and seeing which refusals actually read well in practice. My guess is at least two of the messages I'm proud of today turn out to be useless in context.

Top comments (0)