Make spec-detected auth methods immutable in the add flow

RhysSullivan · RhysSullivan · commit c9bb8eaacfe9 · 2026-06-28T12:25:18.000-07:00
A method seeded from spec/probe detection rendered the full None/API key/
OAuth selector and editable fields, so a user could silently switch a
detected method (e.g. a Bearer-token API) to OAuth with empty endpoints and
end up with a broken integration.

Detected methods now render read-only in a disabled state (lock icon, muted
non-interactive block, "Pulled from spec. Remove to override."). The only
action is to remove the row and add a custom one; hand-added methods stay
fully editable. This lives in the shared AuthMethodListEditor, so the OpenAPI
and MCP add flows both get it. Detection is keyed off an explicit row flag
rather than the seed slug, since MCP seeds a detected method without one.
diff --git a/e2e/selfhost/detected-auth-immutable-ui.test.ts b/e2e/selfhost/detected-auth-immutable-ui.test.ts
@@ -0,0 +1,177 @@
+// Selfhost-only (browser): a spec/probe-DETECTED auth method is immutable in
+// the add flow. The shared AuthMethodListEditor renders detected methods as a
+// disabled, read-only summary ("Pulled from spec. Remove to override.") with no
+// kind selector, so a user can't silently retype the spec's method into a kind
+// nothing backs. A method the user adds by hand stays fully editable. Both the
+// MCP and OpenAPI add flows compose the same editor, so one behavior, two
+// surfaces. Selfhost runs with EXECUTOR_ALLOW_LOCAL_NETWORK so the probe/analyze
+// can reach the loopback fixtures. Video is the artifact.
+import { randomBytes } from "node:crypto";
+import { createServer } from "node:http";
+
+import { expect } from "@effect/vitest";
+import { Effect } from "effect";
+import { makeGreetingMcpServer, serveMcpServerWithOAuth } from "@executor-js/plugin-mcp/testing";
+import { OAuthTestServer } from "@executor-js/sdk/testing";
+
+import { scenario } from "../src/scenario";
+import { Browser, Target } from "../src/services";
+
+const REMOVE_HINT = "Pulled from spec. Remove to override.";
+
+scenario(
+  "Detected auth · an MCP probe's OAuth method is immutable in the add flow",
+  {},
+  Effect.scoped(
+    Effect.gen(function* () {
+      const target = yield* Target;
+      const browser = yield* Browser;
+      // OAuth-protected server: the probe 401s with resource metadata, so the
+      // method list seeds a single detected OAuth row (discovered metadata).
+      const server = yield* serveMcpServerWithOAuth(
+        () => makeGreetingMcpServer({ name: `oauth-mcp-${randomBytes(3).toString("hex")}` }),
+        { path: "/mcp" },
+      );
+      const identity = yield* target.newIdentity();
+
+      yield* browser.session(identity, async ({ page, step }) => {
+        await step("Open the add-MCP flow pointed at the OAuth server", async () => {
+          await page.goto(`/integrations/add/mcp?url=${encodeURIComponent(server.endpoint)}`, {
+            waitUntil: "networkidle",
+          });
+          await page.getByText("How does this server authenticate?").waitFor();
+          await page.getByText("Method 1 · Detected").waitFor();
+        });
+
+        await step("The detected method is locked: read-only, no kind selector", async () => {
+          // Discovered-OAuth summary + the override hint, both inside a disabled
+          // block — and crucially NO editable kind tabs anywhere yet.
+          await page.getByText("OAuth metadata is discovered from this server").waitFor();
+          await page.getByText(REMOVE_HINT).waitFor();
+          expect(
+            await page.locator("[aria-disabled]").count(),
+            "the detected method renders a disabled (non-interactive) block",
+          ).toBeGreaterThan(0);
+          expect(
+            await page.getByText("API key", { exact: true }).count(),
+            "no editable kind selector is shown for the detected method",
+          ).toBe(0);
+        });
+
+        await step("A hand-added method keeps the full editable selector", async () => {
+          await page.getByRole("button", { name: "Add method" }).click();
+          await page.getByText("Method 2").waitFor();
+          // The added row defaults to API key and exposes the None/API key/OAuth
+          // kind tabs — the detected row above still shows none.
+          expect(
+            await page.getByText("API key", { exact: true }).count(),
+            "the added method exposes the kind selector",
+          ).toBeGreaterThan(0);
+          await page.getByText(REMOVE_HINT).waitFor();
+        });
+      });
+    }),
+  ).pipe(Effect.provide(OAuthTestServer.layer())),
+);
+
+/** A real 127.0.0.1 server that serves a static OpenAPI spec for the add flow. */
+const serveSpec = (body: string) =>
+  Effect.acquireRelease(
+    Effect.callback<{ readonly url: string; readonly close: () => void }>((resume) => {
+      const server = createServer((_request, response) => {
+        response.writeHead(200, { "content-type": "application/json" });
+        response.end(body);
+      });
+      server.listen(0, "127.0.0.1", () => {
+        const address = server.address();
+        const port = typeof address === "object" && address ? address.port : 0;
+        resume(
+          Effect.succeed({
+            url: `http://127.0.0.1:${port}/spec.json`,
+            close: () => {
+              server.close();
+              server.closeAllConnections();
+            },
+          }),
+        );
+      });
+    }),
+    (server) => Effect.sync(server.close),
+  );
+
+const apiKeyAndOAuthSpec = (): string =>
+  JSON.stringify({
+    openapi: "3.0.3",
+    info: { title: "Acme Immutable Auth Fixture", version: "1.0.0" },
+    servers: [{ url: "https://api.acme.test" }],
+    security: [{ bearerAuth: [] }, { acmeOAuth: ["read"] }],
+    components: {
+      securitySchemes: {
+        bearerAuth: { type: "http", scheme: "bearer" },
+        acmeOAuth: {
+          type: "oauth2",
+          flows: {
+            authorizationCode: {
+              authorizationUrl: "https://api.acme.test/oauth/authorize",
+              tokenUrl: "https://api.acme.test/oauth/token",
+              scopes: { read: "Read access" },
+            },
+          },
+        },
+      },
+    },
+    paths: {
+      "/widgets": {
+        get: {
+          operationId: "listWidgets",
+          summary: "List widgets",
+          responses: { "200": { description: "ok" } },
+        },
+      },
+    },
+  });
+
+scenario(
+  "Detected auth · OpenAPI spec-detected methods are immutable in the add flow",
+  {},
+  Effect.scoped(
+    Effect.gen(function* () {
+      const target = yield* Target;
+      const browser = yield* Browser;
+      const spec = yield* serveSpec(apiKeyAndOAuthSpec());
+      const identity = yield* target.newIdentity();
+
+      yield* browser.session(identity, async ({ page, step }) => {
+        await step("Analyze a spec that declares both API key and OAuth", async () => {
+          await page.goto(`/integrations/add/openapi`, { waitUntil: "networkidle" });
+          await page.getByPlaceholder(/openapi\.json/i).first().fill(spec.url);
+          await page.getByText("How does this API authenticate?").waitFor();
+          await page.getByText("Method 2").waitFor();
+        });
+
+        await step("Both detected methods are locked, read-only, no kind selector", async () => {
+          // Two detected methods, each with the override hint; the OAuth one
+          // shows the spec's real endpoints read-only.
+          expect(
+            await page.getByText(REMOVE_HINT).count(),
+            "both detected methods show the remove-to-override hint",
+          ).toBe(2);
+          await page.getByText("https://api.acme.test/oauth/authorize").waitFor();
+          expect(
+            await page.getByText("API key", { exact: true }).count(),
+            "no editable kind selector is shown for the detected methods",
+          ).toBe(0);
+        });
+
+        await step("A hand-added method keeps the full editable selector", async () => {
+          await page.getByRole("button", { name: "Add method" }).click();
+          await page.getByText("Method 3").waitFor();
+          expect(
+            await page.getByText("API key", { exact: true }).count(),
+            "the added method exposes the kind selector",
+          ).toBeGreaterThan(0);
+        });
+      });
+    }),
+  ),
+);
diff --git a/packages/react/src/components/auth-method-list-editor.tsx b/packages/react/src/components/auth-method-list-editor.tsx
@@ -12,8 +12,9 @@
 // ---------------------------------------------------------------------------
 
 import { useCallback, useEffect, useRef, useState } from "react";
-import { PlusIcon, XIcon } from "lucide-react";
+import { LockIcon, PlusIcon, XIcon } from "lucide-react";
 
+import { PlacementLine } from "../lib/auth-placements";
 import { Button } from "./button";
 import { FieldLabel } from "./field";
 import {
@@ -35,6 +36,11 @@ export interface AuthMethodSeed {
 
 export interface AuthMethodRow {
   readonly value: AuthTemplateEditorValue;
+  /** True when this row came from detection (a seed), false when the user added
+   *  it. Detected rows are immutable — the spec/probe declared them — so the
+   *  editor renders them read-only. Not inferred from `seedSlug`: some plugins
+   *  (MCP) seed a detected method with a label but no slug. */
+  readonly seeded: boolean;
   readonly seedSlug?: string;
   readonly seedLabel?: string;
 }
@@ -59,6 +65,7 @@ export function useAuthMethodList(seeds: readonly AuthMethodSeed[]): AuthMethodL
       seeds.map(
         (seed: AuthMethodSeed): AuthMethodRow => ({
           value: seed.value,
+          seeded: true,
           ...(seed.slug !== undefined ? { seedSlug: seed.slug } : {}),
           ...(seed.label !== undefined ? { seedLabel: seed.label } : {}),
         }),
@@ -79,7 +86,10 @@ export function useAuthMethodList(seeds: readonly AuthMethodSeed[]): AuthMethodL
   }, []);
 
   const addRow = useCallback(() => {
-    setRows((current: readonly AuthMethodRow[]) => [...current, { value: emptyApiKeyValue() }]);
+    setRows((current: readonly AuthMethodRow[]) => [
+      ...current,
+      { value: emptyApiKeyValue(), seeded: false },
+    ]);
   }, []);
 
   return { rows, setRowAt, removeRowAt, addRow };
@@ -115,36 +125,53 @@ export function AuthMethodListEditor(props: AuthMethodListEditorProps) {
         ) : null
       ) : (
         <div className="flex flex-col gap-3">
-          {list.rows.map((row: AuthMethodRow, index: number) => (
-            <div
-              key={index}
-              className="space-y-2 rounded-lg border border-border/60 bg-muted/20 p-3"
-            >
-              <div className="flex items-center justify-between">
-                <span className="text-xs font-medium text-muted-foreground">
-                  Method {index + 1}
-                  {row.seedLabel ? ` · ${row.seedLabel}` : ""}
-                </span>
-                <Button
-                  type="button"
-                  variant="ghost"
-                  size="icon-sm"
-                  aria-label="Remove method"
-                  className="text-muted-foreground hover:text-foreground"
-                  onClick={() => list.removeRowAt(index)}
-                >
-                  <XIcon />
-                </Button>
+          {list.rows.map((row: AuthMethodRow, index: number) => {
+            // A row seeded from detection is the spec's own auth declaration:
+            // it's IMMUTABLE here. We render it read-only (no kind selector, no
+            // editable fields) so a user can't silently retype the spec's
+            // method into something nothing backs (e.g. flipping a Bearer-token
+            // API to OAuth with empty endpoints). The escape hatch is to remove
+            // the row and add a custom one. Manually added rows (no seed) get
+            // the full editor.
+            const detected = row.seeded;
+            return (
+              <div
+                key={index}
+                className="space-y-2 rounded-lg border border-border/60 bg-muted/20 p-3"
+              >
+                <div className="flex items-center justify-between">
+                  <span className="flex items-center gap-1.5 text-xs font-medium text-muted-foreground">
+                    {detected ? <LockIcon className="size-3 shrink-0" aria-hidden /> : null}
+                    <span>
+                      Method {index + 1}
+                      {row.seedLabel ? ` · ${row.seedLabel}` : ""}
+                    </span>
+                  </span>
+                  <Button
+                    type="button"
+                    variant="ghost"
+                    size="icon-sm"
+                    aria-label="Remove method"
+                    className="text-muted-foreground hover:text-foreground"
+                    onClick={() => list.removeRowAt(index)}
+                  >
+                    <XIcon />
+                  </Button>
+                </div>
+                {detected ? (
+                  <DetectedMethodSummary value={row.value} oauthMetadata={oauthMetadata} />
+                ) : (
+                  <AuthTemplateEditor
+                    value={row.value}
+                    onChange={(next: AuthTemplateEditorValue) => list.setRowAt(index, next)}
+                    {...(allowedKinds ? { allowedKinds } : {})}
+                    {...(presets ? { presets } : {})}
+                    {...(oauthMetadata ? { oauthMetadata } : {})}
+                  />
+                )}
               </div>
-              <AuthTemplateEditor
-                value={row.value}
-                onChange={(next: AuthTemplateEditorValue) => list.setRowAt(index, next)}
-                {...(allowedKinds ? { allowedKinds } : {})}
-                {...(presets ? { presets } : {})}
-                {...(oauthMetadata ? { oauthMetadata } : {})}
-              />
-            </div>
-          ))}
+            );
+          })}
         </div>
       )}
       {list.rows.length > 0 && props.footerHint ? (
@@ -153,3 +180,66 @@ export function AuthMethodListEditor(props: AuthMethodListEditorProps) {
     </section>
   );
 }
+
+/** One read-only `label   value` line, mono value, for the detected summary. */
+function SpecField(props: { readonly label: string; readonly value: string }) {
+  return (
+    <div className="flex gap-2 text-xs">
+      <span className="w-20 shrink-0 text-muted-foreground">{props.label}</span>
+      <span className="break-all font-mono text-foreground/80">{props.value}</span>
+    </div>
+  );
+}
+
+/** Read-only view of a spec-detected method: shows what the spec declared
+ *  (placements / OAuth endpoints) as a DISABLED, non-interactive block. The
+ *  detected method is immutable here, so the summary is styled like a disabled
+ *  field (muted, not-allowed cursor, text not selectable) to communicate that
+ *  plainly. The only action is to remove the row (the header's X) and add a
+ *  custom method to override. */
+function DetectedMethodSummary(props: {
+  readonly value: AuthTemplateEditorValue;
+  readonly oauthMetadata?: "editable" | "discovered";
+}) {
+  const { value, oauthMetadata } = props;
+  return (
+    <div className="space-y-2">
+      <div
+        aria-disabled
+        className="cursor-not-allowed select-none space-y-1 rounded-md border border-border/60 bg-muted/40 px-3 py-2.5 text-muted-foreground"
+      >
+        {value.kind === "none" && (
+          <p className="text-xs">No credential — tools are callable without an account.</p>
+        )}
+
+        {value.kind === "apikey" &&
+          (value.placements.length > 0 ? (
+            <div className="flex flex-wrap gap-x-4 gap-y-1">
+              {value.placements.map((placement, i: number) => (
+                <PlacementLine key={i} placement={placement} />
+              ))}
+            </div>
+          ) : null)}
+
+        {value.kind === "oauth" &&
+          (oauthMetadata === "discovered" ? (
+            <p className="text-xs">
+              OAuth metadata is discovered from this server when you connect an account.
+            </p>
+          ) : (
+            <div className="space-y-1">
+              {value.authorizationUrl ? (
+                <SpecField label="Authorize" value={value.authorizationUrl} />
+              ) : null}
+              {value.tokenUrl ? <SpecField label="Token" value={value.tokenUrl} /> : null}
+              {value.scopes.length > 0 ? (
+                <SpecField label="Scopes" value={value.scopes.join(", ")} />
+              ) : null}
+            </div>
+          ))}
+      </div>
+
+      <p className="text-[11px] text-muted-foreground">Pulled from spec. Remove to override.</p>
+    </div>
+  );
+}
