diff --git a/src/client/harness-client.ts b/src/client/harness-client.ts index 18ace3e05..32d9ca4cc 100644 --- a/src/client/harness-client.ts +++ b/src/client/harness-client.ts @@ -65,12 +65,33 @@ function isHtmlBody(body: string): boolean { return /^\s*, isFme: boolean): void { + private applyDefaultAuth(headers: Record, isFme: boolean, path = ""): void { if (isFme) { // FME/Split Admin APIs expect Bearer auth. Drop x-api-key here so // placeholder credentials are never forwarded to api.split.io. @@ -231,13 +260,23 @@ export class HarnessClient { return; } - // Preserve caller-provided auth instead of layering fallback credentials on top. - if (getHeaderValue(headers, "authorization")) return; - - // Non-FME Harness services continue to use the standard API-key header. + // Always ensure x-api-key is present for Harness APIs — even when Authorization + // is already set. Skipping the key left CG Manager (delegate-setup / delegate-token-ng) + // with only an inter-service JWT that NG Manager accepts but CG rejects → 401. if (!getHeaderValue(headers, "x-api-key")) { headers["x-api-key"] = this.token; } + + // CG Manager accepts x-api-key and Bearer. Replace incompatible Authorization + // schemes (e.g. "genaiservice ") with Bearer from the configured API key + // so CG does not fail closed on the first auth header it tries. + if (isCgManagerPath(path) && this.token && !isPlaceholderCredential(this.token)) { + const auth = getHeaderValue(headers, "authorization"); + if (!auth || !/^Bearer\s+\S+/i.test(auth.trim())) { + deleteHeaderValues(headers, "authorization"); + headers["Authorization"] = `Bearer ${this.token}`; + } + } } /** @@ -349,9 +388,13 @@ export class HarnessClient { } const rawMessage = isGarbageMessage(parsed.message) - ? humanizeHttpError(response.status, body) + ? humanizeHttpError(response.status, body, options.path) : parsed.message!; - const message = enrichErrorMessage(rawMessage, parsed, options.path); + const message = withCgManager401Hint( + enrichErrorMessage(rawMessage, parsed, options.path), + response.status, + options.path, + ); log.debug(`HTTP ${response.status} error`, { body: this.logUnsafeBodies ? body.slice(0, 1000) : redactJsonString(body), }); @@ -493,9 +536,13 @@ export class HarnessClient { try { parsed = JSON.parse(body); } catch { /* non-JSON */ } const rawMessage = isGarbageMessage(parsed.message) - ? humanizeHttpError(response.status, body) + ? humanizeHttpError(response.status, body, options.path) : parsed.message!; - const message = enrichErrorMessage(rawMessage, parsed, options.path); + const message = withCgManager401Hint( + enrichErrorMessage(rawMessage, parsed, options.path), + response.status, + options.path, + ); const error = new HarnessApiError(message, response.status, parsed.code, parsed.correlationId); if ( diff --git a/tasks/lessons.md b/tasks/lessons.md index 43d53d01b..5bf8e086b 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -1,5 +1,11 @@ # Lessons Learned +## Delegate APIs Are CG Manager — Auth Scheme Matters +- **Issue**: `harness_list(resource_type="delegate")` returns HTTP 401 while other NG resources succeed with the same MCP session. Agents/users are told the API key is invalid even when the key works elsewhere. +- **Root cause**: `/ng/api/delegate-setup` and `/ng/api/delegate-token-ng` are ingress-routed to CG Manager (harness-manager), which accepts only `x-api-key` / Bearer (and unused IdentityService+`X-Identity-User`). Inter-service JWTs that NG Manager accepts are rejected. Separately, `applyDefaultAuth` skips injecting `x-api-key` whenever any `Authorization` header is already present. +- **Fix direction**: Dual-send `x-api-key` even when `Authorization` exists (non-FME); optionally Bearer for CG path prefixes; clarify 401 copy for those paths. AskAI/internal still needs a CG-valid credential if it only has a genaiservice JWT. +- **Rule**: Before treating a delegate/token 401 as a bad PAT, check whether the request reached CG Manager with `x-api-key` or Bearer. Do not assume all `/ng/api/*` paths share NG Manager auth. + ## List-Filter Enums Must Be Canonicalized at Dispatch - **Issue**: `listFilterFields.enum` is only visible via `harness_describe`. The global `harness_list` schema cannot encode per-resource enums, so agents often send lowercase (`pending`) while APIs require PascalCase/UPPERCASE. Those 400s count as `tool_error` and can page on-call. - **Fix**: `canonicalizeListFilterEnums` in `Registry.dispatch` rewrites case-insensitive matches to declared enum values (including comma-separated tokens). Also clarify that some resources have a lower `size` max than the global 1–100 tool schema. diff --git a/tasks/todo.md b/tasks/todo.md index 914ad248e..de26dedac 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,5 +1,69 @@ # Harness MCP Server — Task Tracking +## Delegate list 401 / auth not taking API key (this session) + +### Status +- [x] Reproduce: `harness_list(delegate)` → 401 while project/org/secret/connector/user succeed on same MCP session +- [x] Trace root cause via DEL-2489 / AIPLAT-601 / CG Manager auth notes +- [x] Draft fix plan (below) +- [x] Implement client auth fix + focused regressions +- [x] Improve 401 messaging for CG-backed paths +- [ ] Smoke-verify `delegate` + `delegate_token` with a real PAT against `app.harness.io` +- [ ] Note AskAI/mcpServerInternal follow-up if still 401 after OSS dual-auth + +### Implemented +- `applyDefaultAuth` always injects `x-api-key` for non-FME (dual-send with existing Authorization). +- CG Manager paths replace non-Bearer Authorization with `Bearer ` when the configured key is not a placeholder. +- 401 humanization + JSON message enrichment mention CG Manager auth requirements for delegate routes. +- Focused client regressions cover dual-send, CG Bearer rewrite, placeholder non-rewrite, and 401 copy. + +### Why it fails +`/ng/api/delegate-setup/*` and `/ng/api/delegate-token-ng/*` are **not NG Manager**. Ingress maps them to **CG Manager (harness-manager)**. + +CG Manager only accepts: +- `x-api-key` (NG or CG API key / PAT / SAT) +- `Authorization: Bearer ` +- IdentityService token + `X-Identity-User` (unused in practice) + +It **rejects** inter-service JWTs such as `Authorization: genaiservice ` that NG Manager accepts. That is why other NG resources work and delegates alone return 401 — the API key/session credential is effectively not applied in a form CG accepts. + +Related: [AIPLAT-601](https://harness.atlassian.net/browse/AIPLAT-601) (Blocked), [DEL-2489](https://harness.atlassian.net/browse/DEL-2489), Slack `#sme-harness-ai` 2026-08-16 customer report. + +### Amplifying bug in this repo +`HarnessClient.applyDefaultAuth` (non-FME path): + +```ts +if (getHeaderValue(headers, "authorization")) return; // skips x-api-key entirely +if (!getHeaderValue(headers, "x-api-key")) { + headers["x-api-key"] = this.token; +} +``` + +Whenever any `Authorization` header is already present, configured `HARNESS_API_KEY` is never sent as `x-api-key`. NG Manager may still authorize the request; CG Manager returns 401. Error copy then misleadingly says the API key is invalid/expired. + +### Plan +1. **Client dual-auth for non-FME (preferred minimal fix)** + In `applyDefaultAuth`, still inject `x-api-key` from `this.token` when missing, even if `Authorization` is already set. Preserve caller `Authorization`; do not overwrite it. Keep FME Bearer behavior unchanged. +2. **Optional CG hardening for delegate paths** + For `/ng/api/delegate-setup` and `/ng/api/delegate-token-ng`, if only a PAT/SAT is available and no Bearer is set, also send `Authorization: Bearer ` (CG accepts both). Gate behind path prefix or a small `authStyle: "cg_manager"` on those EndpointSpecs — avoid global Bearer injection. +3. **Clearer 401s** + When path matches delegate CG prefixes, enrich the humanized 401 to mention CG Manager requires `x-api-key` or Bearer (not inter-service JWT), so AskAI stops telling customers to “set HARNESS_API_KEY” when the real issue is auth scheme. +4. **Regressions** + - `harness-client.test.ts`: Authorization present + configured token → both `Authorization` and `x-api-key` sent + - Delegate dispatch mock: list request headers include `x-api-key` + - Preserve existing “caller-provided x-api-key casing” and FME tests +5. **Out of scope / platform follow-up** + AskAI `mcpServerInternal` direct-to-CG calls with only `genaiservice` JWT still need a platform credential that CG accepts (real PAT/SAT as x-api-key/Bearer, or CG inter-service support). OSS dual-auth fixes the “Authorization present → drop API key” hole; it cannot invent a CG-valid credential if none exists. +6. **Secondary check while in toolset** + `delegate_token` create OpenAPI wants `tokenName` as a **query** param; current bodyBuilder sends `{ name }`. Verify and fix in the same PR only if it fails independently of auth. + +### Non-goals +- Do not change the public 11-tool contract +- Do not invent a new NG-only delegate list API (none documented) +- Do not weaken fail-open / multi-user credential isolation + +--- + ## OPA policy multi-scope (this session) - [x] Add `supportedScopes: ["account", "org", "project"]` + description hints for `policy` and `policy_set` - [x] Extend governance tests for supportedScopes and `resource_scope` query-param dispatch diff --git a/tests/client/harness-client.test.ts b/tests/client/harness-client.test.ts index dde7aaa72..e783eaf91 100644 --- a/tests/client/harness-client.test.ts +++ b/tests/client/harness-client.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { HarnessClient } from "../../src/client/harness-client.js"; +import { HarnessClient, isCgManagerPath } from "../../src/client/harness-client.js"; import { HarnessApiError } from "../../src/utils/errors.js"; import type { Config } from "../../src/config.js"; @@ -23,6 +23,16 @@ function makeConfig(overrides: Partial = {}): Config { }; } +describe("isCgManagerPath", () => { + it("matches delegate-setup and delegate-token-ng paths", () => { + expect(isCgManagerPath("/ng/api/delegate-setup/listDelegates")).toBe(true); + expect(isCgManagerPath("/ng/api/delegate-token-ng")).toBe(true); + expect(isCgManagerPath("/ng/api/delegate-token-ng/my-token")).toBe(true); + expect(isCgManagerPath("/ng/api/projects")).toBe(false); + expect(isCgManagerPath("/ng/api/connectors/listV2")).toBe(false); + }); +}); + describe("HarnessClient", () => { let fetchSpy: ReturnType; @@ -367,7 +377,7 @@ describe("HarnessClient", () => { expect(headers["Harness-Account"]).toBe("test-account"); }); - it("preserves caller-provided non-FME auth regardless of header casing", async () => { + it("dual-sends x-api-key when caller already provided Authorization", async () => { fetchSpy.mockResolvedValue(new Response(JSON.stringify({}), { status: 200 })); const client = new HarnessClient(makeConfig()); @@ -379,7 +389,93 @@ describe("HarnessClient", () => { const init = fetchSpy.mock.calls[0][1] as RequestInit; const headers = new Headers(init.headers); expect(headers.get("Authorization")).toBe("Bearer session-token"); - expect(headers.has("x-api-key")).toBe(false); + expect(headers.get("x-api-key")).toBe("pat.test-account.token.secret"); + }); + + it("replaces non-Bearer Authorization with Bearer+x-api-key on CG Manager delegate paths", async () => { + fetchSpy.mockResolvedValue(new Response(JSON.stringify({ resource: [] }), { status: 200 })); + const client = new HarnessClient(makeConfig()); + + await client.request({ + method: "POST", + path: "/ng/api/delegate-setup/listDelegates", + headers: { Authorization: "genaiservice service-jwt" }, + body: { filterType: "Delegate" }, + }); + + const init = fetchSpy.mock.calls[0][1] as RequestInit; + const headers = new Headers(init.headers); + expect(headers.get("Authorization")).toBe("Bearer pat.test-account.token.secret"); + expect(headers.get("x-api-key")).toBe("pat.test-account.token.secret"); + }); + + it("preserves existing Bearer Authorization on CG Manager paths while still sending x-api-key", async () => { + fetchSpy.mockResolvedValue(new Response(JSON.stringify({ resource: [] }), { status: 200 })); + const client = new HarnessClient(makeConfig()); + + await client.request({ + method: "GET", + path: "/ng/api/delegate-token-ng", + headers: { Authorization: "Bearer session-pat" }, + }); + + const init = fetchSpy.mock.calls[0][1] as RequestInit; + const headers = new Headers(init.headers); + expect(headers.get("Authorization")).toBe("Bearer session-pat"); + expect(headers.get("x-api-key")).toBe("pat.test-account.token.secret"); + }); + + it("adds Bearer from API key on CG Manager paths when Authorization is missing", async () => { + fetchSpy.mockResolvedValue(new Response(JSON.stringify({ resource: [] }), { status: 200 })); + const client = new HarnessClient(makeConfig()); + + await client.request({ + method: "POST", + path: "/ng/api/delegate-setup/listDelegates", + body: { filterType: "Delegate" }, + }); + + const init = fetchSpy.mock.calls[0][1] as RequestInit; + const headers = new Headers(init.headers); + expect(headers.get("Authorization")).toBe("Bearer pat.test-account.token.secret"); + expect(headers.get("x-api-key")).toBe("pat.test-account.token.secret"); + }); + + it("does not replace non-Bearer Authorization on CG paths when API key is a placeholder", async () => { + fetchSpy.mockResolvedValue(new Response(JSON.stringify({ resource: [] }), { status: 200 })); + const client = new HarnessClient(makeConfig({ HARNESS_API_KEY: "pat.internal.internal.dummy" })); + + await client.request({ + method: "POST", + path: "/ng/api/delegate-setup/listDelegates", + headers: { Authorization: "genaiservice service-jwt" }, + body: { filterType: "Delegate" }, + }); + + const init = fetchSpy.mock.calls[0][1] as RequestInit; + const headers = new Headers(init.headers); + expect(headers.get("Authorization")).toBe("genaiservice service-jwt"); + expect(headers.get("x-api-key")).toBe("pat.internal.internal.dummy"); + }); + + it("humanizes CG Manager 401s with delegate auth guidance", async () => { + fetchSpy.mockResolvedValue(new Response("unauthorized", { status: 401 })); + const client = new HarnessClient(makeConfig()); + + await expect( + client.request({ method: "POST", path: "/ng/api/delegate-setup/listDelegates" }), + ).rejects.toThrow(/CG Manager/); + }); + + it("appends CG Manager hint to JSON 401 messages on delegate paths", async () => { + fetchSpy.mockResolvedValue( + new Response(JSON.stringify({ message: "Invalid credentials" }), { status: 401 }), + ); + const client = new HarnessClient(makeConfig()); + + await expect( + client.request({ method: "GET", path: "/ng/api/delegate-token-ng" }), + ).rejects.toThrow(/Invalid credentials.*CG Manager/); }); it("does not inject x-api-key when caller already provided it with alternate casing", async () => {