diff --git a/CLAUDE.md b/CLAUDE.md index 6102f28..0c28e7a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,6 +27,7 @@ A Model Context Protocol (MCP) server that exposes the Kosli API to LLM clients - ESM only (`"type": "module"`, `NodeNext` resolution). Relative imports must use `.js` extensions even though the source is `.ts`. - TypeScript `strict: true`. Don't weaken it. - The `org` path parameter falls back to `config.org` (from `KOSLI_ORG`). Preserve this in `KosliClient.buildUrl`. +- A per-call org is interpreted in exactly one place: `normalizeOrg` + `orgError` in `src/tools/execute-action.ts`, which both execute tools go through (the `KOSLI_ORG` default never passes through them: `loadConfig` trims it and rejects a blank one at startup, and `KosliClient.buildUrl` falls back to it when a call names no org). The `org` tool input and `params.org` obey the same rules — trimmed, only a string names one (coercing would turn `["a", "b"]` into the org `a,b`), `null` means not supplied, blank rejected, an org on a non-org-scoped action rejected, and two different orgs in one call rejected rather than resolved, because the same path performs writes. Note the ordering: `params.org` is read *before* `unwrapBodyParam` and the body's org *after* it — the unwrap flattens a write's body over the top level, so reading only once lets the spread pick a winner and hides the disagreement. Keep that decision in the one place; don't re-implement it per tool in `index.ts`. - Errors from the Kosli API are returned as `{ error: true, status, statusText, message }` — not thrown. Tools stringify whatever they get. Keep this contract; the LLM handles the error object. - Responses are serialized with `JSON.stringify(result)` (compact, no pretty-printing) to minimize token usage. Don't revert to pretty-printing. - All API requests include `User-Agent: kosli-mcp-server/` for server-side tracking, with the version coming from `VERSION` in `src/version.ts`. Preserve the header and keep the version in it — the backend uses it to tell releases apart. diff --git a/README.md b/README.md index a449694..7f2aaee 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ The server reads configuration from environment variables: | Variable | Required | Default | Notes | |----------|----------|---------|-------| | `KOSLI_API_TOKEN` | yes | — | Preferred. `KOSLI_API_KEY` is accepted as a fallback. | -| `KOSLI_ORG` | yes | — | Default org used when a path param `org` is not supplied. | +| `KOSLI_ORG` | yes | — | Default org used when a call does not supply one. Trimmed; whitespace-only is rejected at startup. | | `KOSLI_BASE_URL` | no | `https://app.kosli.com` | EU (default), US (`https://app.us.kosli.com`), or your single-tenant endpoint. | ## Wire up to an MCP client @@ -107,7 +107,38 @@ Typical LLM flow: > [!IMPORTANT] > `execute_write_action` creates, modifies, and deletes real resources in your Kosli organization. MCP clients gate these calls behind an approval prompt — read the action ID and parameters before approving. An LLM may select the wrong action, or the right action with the wrong parameters, and approval is the only checkpoint before the call is made. Treat deletions and anything touching service accounts or API keys with particular care. -The `org` path parameter defaults to `KOSLI_ORG` if not supplied. For `GET`/`DELETE`, non-path params become query parameters; for other methods they become the JSON body. +For `GET`/`DELETE`, non-path params become query parameters; for other methods they become the JSON body. + +### Working with more than one organization + +Both execute tools take an optional `org`, so a single running server can reach +any organization your token has access to — useful when you are moving between +orgs in one conversation rather than restarting the server for each: + +```json +{ "actionId": "list_envs", "org": "cyber-dojo", "fields": ["name"] } +``` + +- Omit it and the call goes to `KOSLI_ORG`. +- It applies to that one call. The next call goes back to `KOSLI_ORG`. +- Surrounding whitespace is trimmed. A blank org is rejected rather than + quietly producing a URL with the organization missing from it. +- One org, named as a string. Anything else is rejected rather than coerced, + because coercion turns a value that is not a name into one that looks like a + name: `["cyber-dojo", "kosli-public"]` would request the org + `cyber-dojo,kosli-public`, and an org id would request an org called `1234`. +- Four actions are not organization-scoped (`get_user_default_org`, + `list_system_attestation_types`, and the two `/schemas/...` actions). Passing + an org to one of those is rejected rather than ignored, whether it arrives as + `org` or inside `params`. +- There are three ways to name an org in one call: this input, `params.org`, and + an `org` inside a write's request body. They must agree; two different names + are rejected rather than resolved, because the same tools perform writes and + the target is never guessed. +- An org your token cannot reach comes back as the usual error object carrying + the API's status — `403` for both an org you are not a member of and one that + does not exist. Note that Kosli staff accounts can read any org, so a + successful read does not prove a write would be permitted. Both execute tools accept an optional `fields` array to request only specific top-level fields from each object in the response. This dramatically reduces response size and token usage: diff --git a/src/config.ts b/src/config.ts index 74f92a1..0447503 100644 --- a/src/config.ts +++ b/src/config.ts @@ -6,9 +6,11 @@ export function loadConfig(): Config { throw new Error("KOSLI_API_TOKEN (or KOSLI_API_KEY) environment variable is required"); } - const org = process.env.KOSLI_ORG; + // Trimmed so the existing check also catches a whitespace-only value, which + // is truthy and would otherwise become the org path segment on every call. + const org = process.env.KOSLI_ORG?.trim(); if (!org) { - throw new Error("KOSLI_ORG environment variable is required"); + throw new Error("KOSLI_ORG environment variable is required, and must not be blank"); } const baseUrl = process.env.KOSLI_BASE_URL || "https://app.kosli.com"; diff --git a/src/index.ts b/src/index.ts index 2f3a35b..4c721b6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,6 +13,11 @@ import { VERSION } from "./version.js"; const entries = catalog as CatalogEntry[]; const config = loadConfig(); +// Shared by both execute tools so the two can't drift apart. +const ORG_INPUT = z.string().optional().describe( + `Kosli organization to run this call against, e.g. "cyber-dojo". Defaults to "${config.org}" (from KOSLI_ORG). It applies to this call only — the next call goes back to the default unless you set it again.`, +); + const server = new McpServer({ name: "kosli", version: VERSION, @@ -48,18 +53,19 @@ server.registerTool( "execute_read_action", { title: "Execute read action", - description: `Execute a read-only (GET) Kosli API action by its ID with the given parameters. Use search_actions first to find the action ID and required parameters. The 'org' parameter defaults to "${config.org}" — you do not need to supply it unless querying a different organization.`, + description: `Execute a read-only (GET) Kosli API action by its ID with the given parameters. Use search_actions first to find the action ID and required parameters. The call runs against the "${config.org}" organization unless you set 'org'.`, annotations: { readOnlyHint: true, }, inputSchema: { actionId: z.string().describe("The action ID from search_actions results"), + org: ORG_INPUT, params: z.record(z.string(), z.unknown()).optional().default({}).describe("Parameters for the action (path params or query params)"), fields: z.array(z.string()).optional().describe("Only include these fields in each object of the response. Dramatically reduces response size. Example: [\"name\",\"compliant\",\"fingerprint\",\"reasons_for_incompliance\"]"), }, }, - async ({ actionId, params, fields }) => { - const result = await executeAction(entries, config, actionId, params, fields, undefined, "GET"); + async ({ actionId, params, fields, org }) => { + const result = await executeAction(entries, config, actionId, params, fields, undefined, "GET", org); return { content: [ { @@ -75,19 +81,20 @@ server.registerTool( "execute_write_action", { title: "Execute write action", - description: `Execute a write (POST, PUT, PATCH, DELETE) Kosli API action by its ID with the given parameters. Use search_actions first to find the action ID and required parameters. The 'org' parameter defaults to "${config.org}" — you do not need to supply it unless querying a different organization.`, + description: `Execute a write (POST, PUT, PATCH, DELETE) Kosli API action by its ID with the given parameters. Use search_actions first to find the action ID and required parameters. The call runs against the "${config.org}" organization unless you set 'org' — check it before approving, the write lands in whichever org it names.`, annotations: { destructiveHint: true, readOnlyHint: false, }, inputSchema: { actionId: z.string().describe("The action ID from search_actions results"), + org: ORG_INPUT, params: z.record(z.string(), z.unknown()).optional().default({}).describe("Parameters for the action (path params, query params, or body)"), fields: z.array(z.string()).optional().describe("Only include these fields in each object of the response. Dramatically reduces response size."), }, }, - async ({ actionId, params, fields }) => { - const result = await executeAction(entries, config, actionId, params, fields, undefined, "WRITE"); + async ({ actionId, params, fields, org }) => { + const result = await executeAction(entries, config, actionId, params, fields, undefined, "WRITE", org); return { content: [ { diff --git a/src/tools/execute-action.ts b/src/tools/execute-action.ts index ce16dce..5008a24 100644 --- a/src/tools/execute-action.ts +++ b/src/tools/execute-action.ts @@ -56,6 +56,60 @@ function unwrapBodyParam( return { ...rest, ...(body as Record) }; } +/** + * The org a call is aimed at, as it will actually be used: trimmed, and + * `undefined` when nothing was supplied. `null` counts as nothing, matching + * `KosliClient.buildUrl`, which falls back to `config.org` on a nullish + * `params.org`. An empty string means "supplied, but unusable", which + * `orgError` turns into a rejection. + */ +function normalizeOrg(value: unknown): string | undefined { + if (value === undefined || value === null) return undefined; + // Only a string can name an org. `params` is a record of `unknown`, so the + // model can put anything here, and coercing it would turn a value that is not + // a name into one that looks like a name: `["a", "b"]` reads as the org "a,b" + // and an org id reads as an org called "1234". Both would then pass the + // agreement check below as a single name. Hand back the unusable marker + // instead and let orgError say so. + if (typeof value !== "string") return ""; + return value.trim(); +} + +/** + * Check the orgs a call names, returning a message when the request cannot be + * honoured as written. This is the only place the target org is decided, so + * both execute tools behave identically and every way of naming one obeys the + * same rules. + * + * `named` holds the distinct orgs the call mentions, already normalized. There + * are three ways to name one and they must agree: the `org` argument, + * `params.org`, and — for a write — an `org` inside the request body, which + * `unwrapBodyParam` flattens over the top level. Two different names are + * rejected rather than resolved: the same code path performs writes, and + * guessing wrong would write to the wrong organization. + */ +function orgError(entry: CatalogEntry, named: string[]): string | undefined { + const takesOrg = entry.parameters.some((p) => p.name === "org" && p.in === "path"); + if (!takesOrg) { + if (named.length === 0) return undefined; + return `Action "${entry.id}" is not organization-scoped — it takes no org. Retry with no org in the org parameter, in params.org, or in the request body.`; + } + + // Before any disagreement: an unusable value is the thing to report, and + // reporting it as a nameless org disagreeing with a real one would send the + // caller to drop one of the two rather than to fix the bad value. + if (named.includes("")) { + return "The org must be a single organization name, given as a non-empty string. Check the org parameter, params.org, and any org in the request body, or omit all of them to use the configured default."; + } + + if (named.length > 1) { + const quoted = named.map((o) => `"${o}"`).join(" and "); + return `Conflicting orgs in one call: ${quoted}. The org parameter, params.org, and an org in the request body must agree — supply just one.`; + } + + return undefined; +} + export async function executeAction( catalog: CatalogEntry[], config: Config, @@ -64,6 +118,7 @@ export async function executeAction( fields?: string[], fetchFn: FetchFn = globalThis.fetch, mode?: ToolMode, + org?: string, ): Promise { const entry = catalog.find((e) => e.id === actionId); if (!entry) { @@ -84,8 +139,32 @@ export async function executeAction( }; } + // Collect the org from every channel before deciding. `unwrapBodyParam` + // flattens a write's request body over the top level, so params.org has to be + // read before the unwrap and the body's org after it — otherwise the spread + // picks a winner and the disagreement is never seen. When the unwrap declines, + // `unwrapped` is `params` and the body's org is never read. That is correct: + // an unflattened body cannot reach the path, so there is nothing to check. + const beforeUnwrap = normalizeOrg(params.org); + const unwrapped = unwrapBodyParam(entry, params); + const named = [...new Set( + [normalizeOrg(org), beforeUnwrap, normalizeOrg(unwrapped.org)] + .filter((o): o is string => o !== undefined), + )]; + + const message = orgError(entry, named); + if (message) return { error: true, message }; + + // The checked value is the one that goes out, so buildUrl cannot see a + // different org from the one that was approved. With nothing named, the key + // is dropped rather than left as a null that would ride along as a stray + // query parameter. + const withOrg = { ...unwrapped }; + if (named.length === 0) delete withOrg.org; + else withOrg.org = named[0]; + const client = new KosliClient(config, fetchFn); - const result = await client.execute(entry, unwrapBodyParam(entry, params)); + const result = await client.execute(entry, withOrg); // Never strip error shapes — pickFields would reduce them to {} // and hide the failure from the LLM. diff --git a/test/catalog.test.ts b/test/catalog.test.ts index 6342b46..8a1a2fa 100644 --- a/test/catalog.test.ts +++ b/test/catalog.test.ts @@ -17,4 +17,148 @@ describe("catalog.json", () => { const ids = catalog.map((entry) => entry.id); expect(new Set(ids).size).toBe(ids.length); }); + + // README names these four as the actions that reject an `org`. If the spec + // gains or loses one, update the README in the same PR as the regenerated + // catalog — otherwise the docs quietly start lying. + it("has exactly the four documented actions that take no org", () => { + const withoutOrg = catalog + .filter((entry) => !entry.parameters.some((p) => p.name === "org" && p.in === "path")) + .map((entry) => entry.id) + .sort(); + + expect(withoutOrg).toEqual([ + "environment_policy_schema_v1", + "flow_template_schema_v1", + "get_user_default_org", + "list_system_attestation_types", + ]); + }); + + // buildUrl only fills `{org}` from a path parameter; an org arriving any + // other way would bypass the checks in executeAction. + it("never declares org as a query or header parameter", () => { + const offenders = catalog + .filter((entry) => entry.parameters.some((p) => p.name === "org" && p.in !== "path")) + .map((entry) => entry.id); + + expect(offenders).toEqual([]); + }); + + // executeAction reads an `org` inside a write's request body as the target + // org: unwrapBodyParam flattens the body over the top level, so the value + // moves into the path segment and leaves the payload. A body field genuinely + // called "org" and meaning something else would be hijacked that way. + it("never declares org as a request body property", () => { + const offenders = catalog + .filter((entry) => entry.requestBody?.some((body) => bodyFields(body.schema).includes("org"))) + .map((entry) => entry.id); + + expect(offenders).toEqual([]); + }); +}); + +// The field names a request body contributes at its top level, which are the +// ones unwrapBodyParam can turn into params — it flattens exactly one level, so +// a field nested inside another object is not one of them. A schema can name +// them directly, through `required`, or through composition, and the generator +// copies composition through untouched (see scripts/resolve-refs.ts, and the +// `allOf` already on create_artifact and post_override_attestation). +// +// `not` is deliberately not walked: a branch saying a field must be ABSENT does +// not name it. Known limit: a `patternProperties` at a body root whose pattern +// happens to admit "org" names the field by pattern rather than by name, so it +// is not collected. The catalog's 22 uses all sit inside nested property +// values, which are out of scope anyway. +function bodyFields(schema: unknown): string[] { + if (schema === null || typeof schema !== "object") return []; + const node = schema as Record; + + const declared = + node.properties !== null && typeof node.properties === "object" + ? Object.keys(node.properties as Record) + : []; + + const composed = ["allOf", "anyOf", "oneOf", "if", "then", "else"].flatMap((keyword) => { + const branch = node[keyword]; + return Array.isArray(branch) ? branch.flatMap(bodyFields) : bodyFields(branch); + }); + + // These map a trigger field to what it pulls in. Both halves name top-level + // fields: the key is the trigger, which is a field of the body by definition, + // and the value is either a schema or (draft-07 `dependencies`, and + // `dependentRequired`) a bare list of field names. + const dependent = ["dependentSchemas", "dependentRequired", "dependencies"].flatMap((keyword) => { + const branch = node[keyword]; + if (branch === null || typeof branch !== "object" || Array.isArray(branch)) return []; + const map = branch as Record; + return [...Object.keys(map), ...Object.values(map).flatMap(dependentFields)]; + }); + + return [...declared, ...names(node.required), ...composed, ...dependent]; +} + +function dependentFields(value: unknown): string[] { + return Array.isArray(value) ? names(value) : bodyFields(value); +} + +function names(value: unknown): string[] { + return Array.isArray(value) ? value.filter((v): v is string => typeof v === "string") : []; +} + +// bodyFields decides whether the guard above can see a field at all. The walk +// does run on today's catalog, but every name it reaches is already in the root +// `properties`, so removing it changes no verdict and the suite stays green. +// These tests pin each branch directly, so a later simplification cannot +// silently revert the guard to reading `properties` alone. +describe("bodyFields", () => { + it("collects properties declared directly", () => { + expect(bodyFields({ properties: { org: {}, flow_name: {} } })).toContain("org"); + }); + + it("collects a name that only appears in required", () => { + expect(bodyFields({ anyOf: [{ required: ["org"] }] })).toContain("org"); + }); + + it.each([ + ["allOf", { allOf: [{ properties: { org: {} } }] }], + ["anyOf", { anyOf: [{ properties: { org: {} } }] }], + ["oneOf", { oneOf: [{ properties: { org: {} } }] }], + ["if", { if: { properties: { org: {} } } }], + ["then", { if: {}, then: { properties: { org: {} } } }], + ["else", { if: {}, else: { properties: { org: {} } } }], + ["nested composition", { allOf: [{ anyOf: [{ properties: { org: {} } }] }] }], + ["dependentSchemas", { dependentSchemas: { fingerprint: { properties: { org: {} } } } }], + ["dependentRequired", { dependentRequired: { fingerprint: ["org"] } }], + ["dependencies as a schema", { dependencies: { fingerprint: { properties: { org: {} } } } }], + ["dependencies as a name list", { dependencies: { fingerprint: ["org"] } }], + ["a dependency trigger key", { dependentRequired: { org: ["scope"] } }], + ])("collects a name reached through %s", (_label, schema) => { + expect(bodyFields(schema)).toContain("org"); + }); + + it("does not collect a field nested inside another object", () => { + expect(bodyFields({ properties: { repo_info: { properties: { org: {} } } } })).not.toContain("org"); + }); + + it("does not collect a field a not branch forbids", () => { + expect(bodyFields({ not: { required: ["org"] } })).not.toContain("org"); + }); + + it("survives shapes the spec should never produce", () => { + const junk = { + properties: null, + required: [1, "org"], + anyOf: "not-an-array", + oneOf: [null, 5, []], + then: 7, + dependencies: ["not", "a", "map"], + dependentSchemas: { a: null, b: "text" }, + }; + + expect(() => bodyFields(junk)).not.toThrow(); + // "org" from required; "a" and "b" are dependency trigger keys, whose + // unusable values contribute nothing further. + expect(bodyFields(junk)).toEqual(["org", "a", "b"]); + }); }); diff --git a/test/config.test.ts b/test/config.test.ts index df040e2..c6b3d21 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -69,6 +69,20 @@ describe("loadConfig", () => { expect(() => loadConfig()).toThrow("KOSLI_ORG"); }); + it("throws when KOSLI_ORG is only whitespace", () => { + process.env.KOSLI_API_KEY = "test-key"; + process.env.KOSLI_ORG = " "; + + expect(() => loadConfig()).toThrow("KOSLI_ORG"); + }); + + it("trims KOSLI_ORG", () => { + process.env.KOSLI_API_KEY = "test-key"; + process.env.KOSLI_ORG = " test-org "; + + expect(loadConfig().org).toBe("test-org"); + }); + it("throws when KOSLI_BASE_URL is not https", () => { process.env.KOSLI_API_KEY = "test-key"; process.env.KOSLI_ORG = "test-org"; diff --git a/test/fixtures/catalog-subset.json b/test/fixtures/catalog-subset.json index a546345..f861167 100644 --- a/test/fixtures/catalog-subset.json +++ b/test/fixtures/catalog-subset.json @@ -125,5 +125,16 @@ { "name": "body", "required": true, "description": "Request body (multipart/form-data)" } ], "searchText": "create or update policy create or update a policy in an organization. put policies org policies" + }, + { + "id": "get_user_default_org", + "method": "GET", + "path": "/user/default-org", + "summary": "Get default organization", + "description": "Get the default org for the current user.", + "tags": ["User"], + "parameters": [], + "requestBody": null, + "searchText": "get default organization get the default org for the current user. get user default-org user" } ] diff --git a/test/tools/execute-action.test.ts b/test/tools/execute-action.test.ts index e659921..a2639ac 100644 --- a/test/tools/execute-action.test.ts +++ b/test/tools/execute-action.test.ts @@ -292,3 +292,365 @@ describe("request body unwrapping", () => { ); }); }); + +describe("org selection", () => { + function mockFetchOk(body: unknown = { ok: true }) { + return vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve(body), + }); + } + + it("targets the org given as the org parameter", async () => { + const mockFetch = mockFetchOk(); + + await executeAction(entries, config, "list_environments", {}, undefined, mockFetch, "GET", "cyber-dojo"); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/cyber-dojo", + expect.anything(), + ); + }); + + it("falls back to the configured org when none is given", async () => { + const mockFetch = mockFetchOk(); + + await executeAction(entries, config, "list_environments", {}, undefined, mockFetch, "GET"); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/test-org", + expect.anything(), + ); + }); + + it("still accepts an org supplied inside params", async () => { + const mockFetch = mockFetchOk(); + + await executeAction(entries, config, "list_environments", { org: "cyber-dojo" }, undefined, mockFetch, "GET"); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/cyber-dojo", + expect.anything(), + ); + }); + + it("accepts the same org in both places", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "list_environments", { org: "cyber-dojo" }, + undefined, mockFetch, "GET", "cyber-dojo", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/cyber-dojo", + expect.anything(), + ); + }); + + it("rejects conflicting orgs without calling the API", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "list_environments", { org: "kosli-public" }, + undefined, mockFetch, "GET", "cyber-dojo", + ); + + expect(result).toEqual({ + error: true, + message: + 'Conflicting orgs in one call: "cyber-dojo" and "kosli-public". The org parameter, params.org, and an org in the request body must agree — supply just one.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("rejects a blank org rather than building a URL with a missing segment", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "list_environments", {}, + undefined, mockFetch, "GET", " ", + ); + + expect(result).toEqual({ + error: true, + message: + "The org must be a single organization name, given as a non-empty string. Check the org parameter, params.org, and any org in the request body, or omit all of them to use the configured default.", + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("rejects an org on an action that is not organization-scoped", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "get_user_default_org", {}, + undefined, mockFetch, "GET", "cyber-dojo", + ); + + expect(result).toEqual({ + error: true, + message: + 'Action "get_user_default_org" is not organization-scoped — it takes no org. Retry with no org in the org parameter, in params.org, or in the request body.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("calls a non-org-scoped action when no org is given", async () => { + const mockFetch = mockFetchOk({ default_org_name: "test-org" }); + + const result = await executeAction(entries, config, "get_user_default_org", {}, undefined, mockFetch, "GET"); + + expect(result).toEqual({ default_org_name: "test-org" }); + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/user/default-org", + expect.anything(), + ); + }); + + it("trims a padded org rather than encoding the padding into the path", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "list_environments", {}, + undefined, mockFetch, "GET", " cyber-dojo ", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/cyber-dojo", + expect.anything(), + ); + }); + + it("applies the same trimming to an org supplied inside params", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "list_environments", { org: " cyber-dojo " }, + undefined, mockFetch, "GET", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/cyber-dojo", + expect.anything(), + ); + }); + + it("rejects a blank org supplied inside params, rather than dropping the path segment", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "list_environments", { org: "" }, + undefined, mockFetch, "GET", + ); + + expect(result).toEqual({ + error: true, + message: + "The org must be a single organization name, given as a non-empty string. Check the org parameter, params.org, and any org in the request body, or omit all of them to use the configured default.", + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("treats a null org in params as not supplied, as buildUrl does", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "list_environments", { org: null }, + undefined, mockFetch, "GET", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/test-org", + expect.anything(), + ); + }); + + it.each([ + ["a list of orgs", ["cyber-dojo", "kosli-public"]], + ["a single-element list", ["cyber-dojo"]], + ["an object", {}], + ["a number, which could name a real org", 1234], + ["a boolean", true], + ])("rejects %s in params rather than coercing it into the path", async (_label, org) => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "list_environments", { org }, + undefined, mockFetch, "GET", + ); + + expect(result).toEqual({ + error: true, + message: + "The org must be a single organization name, given as a non-empty string. Check the org parameter, params.org, and any org in the request body, or omit all of them to use the configured default.", + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("reports the unusable value, not a disagreement, when a real org is named too", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "list_environments", { org: [] }, + undefined, mockFetch, "GET", "cyber-dojo", + ); + + expect(result).toEqual({ + error: true, + message: + "The org must be a single organization name, given as a non-empty string. Check the org parameter, params.org, and any org in the request body, or omit all of them to use the configured default.", + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + // A body-nested org is params.org after the unwrap, so it is a supported way + // to name the target rather than only something to cross-check. The other + // body-org tests pin that the value is READ: removing the post-unwrap read + // turns three of them red. None pinned that it is HONOURED. Keeping the + // cross-check while no longer using a body-only org as the target passed the + // whole suite before this test existed. + it("targets an org named only inside a write's request body", async () => { + const mockFetch = mockFetchOk({ created: true }); + + await executeAction( + entries, config, "post_control", + { body: { org: "cyber-dojo", identifier: "ctrl-1" } }, + undefined, mockFetch, "WRITE", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/controls/cyber-dojo", + expect.objectContaining({ body: JSON.stringify({ identifier: "ctrl-1" }) }), + ); + }); + + it("rejects an org nested in the request body that contradicts the org parameter", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "post_control", + { body: { org: "other-org", identifier: "ctrl-1" } }, + undefined, mockFetch, "WRITE", "cyber-dojo", + ); + + expect(result).toEqual({ + error: true, + message: + 'Conflicting orgs in one call: "cyber-dojo" and "other-org". The org parameter, params.org, and an org in the request body must agree — supply just one.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("rejects an org supplied only inside params for a non-org-scoped action", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "get_user_default_org", { org: "cyber-dojo" }, + undefined, mockFetch, "GET", + ); + + expect(result).toEqual({ + error: true, + message: + 'Action "get_user_default_org" is not organization-scoped — it takes no org. Retry with no org in the org parameter, in params.org, or in the request body.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("rejects a request body org that contradicts params.org, with no org parameter", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "post_control", + { org: "approved-org", body: { org: "other-org", identifier: "ctrl-1" } }, + undefined, mockFetch, "WRITE", + ); + + expect(result).toEqual({ + error: true, + message: + 'Conflicting orgs in one call: "approved-org" and "other-org". The org parameter, params.org, and an org in the request body must agree — supply just one.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("rejects the same contradiction on the multipart write branch", async () => { + const mockFetch = vi.fn(); + + const result = await executeAction( + entries, config, "put_policy", + { org: "approved-org", body: { org: "other-org", name: "policy-1" } }, + undefined, mockFetch, "WRITE", + ); + + expect(result).toEqual({ + error: true, + message: + 'Conflicting orgs in one call: "approved-org" and "other-org". The org parameter, params.org, and an org in the request body must agree — supply just one.', + }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("targets the org on a multipart write without adding it as a form field", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "put_policy", { body: { name: "policy-1" } }, + undefined, mockFetch, "WRITE", "cyber-dojo", + ); + + const [url, init] = mockFetch.mock.calls[0]; + expect(url).toBe("https://app.kosli.com/api/v2/policies/cyber-dojo"); + expect([...(init.body as FormData).keys()]).toEqual(["name"]); + }); + + it("drops a null org instead of forwarding it as a query parameter", async () => { + const mockFetch = mockFetchOk({ default_org_name: "test-org" }); + + await executeAction( + entries, config, "get_user_default_org", { org: null }, + undefined, mockFetch, "GET", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/user/default-org", + expect.anything(), + ); + }); + + // The org is a top-level input on a tool that performs writes, and it lands in + // a path segment. buildUrl percent-encodes it, so a slash-bearing value stays + // one segment instead of re-pointing the request at another endpoint. + // normalizeOrg deliberately does not reject this: the encoding is the defence, + // so pin it here rather than leaving it implicit two files away. + it("encodes an org rather than letting it rewrite the path", async () => { + const mockFetch = mockFetchOk(); + + await executeAction( + entries, config, "list_environments", {}, + undefined, mockFetch, "GET", "../../user/default-org", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/environments/..%2F..%2Fuser%2Fdefault-org", + expect.anything(), + ); + }); + + it("applies to writes, and the org does not leak into the request body", async () => { + const mockFetch = mockFetchOk({ created: true }); + const body = { identifier: "ctrl-1", name: "Control 1" }; + + await executeAction( + entries, config, "post_control", { body }, + undefined, mockFetch, "WRITE", "cyber-dojo", + ); + + expect(mockFetch).toHaveBeenCalledWith( + "https://app.kosli.com/api/v2/controls/cyber-dojo", + expect.objectContaining({ body: JSON.stringify(body) }), + ); + }); +});