From a4c6f150cc2caccf5d8d6922e462b0d9a24eb58b Mon Sep 17 00:00:00 2001 From: Mohammed Naji Date: Fri, 18 Sep 2026 01:27:25 +0400 Subject: [PATCH 1/2] fix(identity): reject control characters in identity fingerprints (#452) An identity fingerprint containing a control character passed configuration validation and passed verification, and was then silently rewritten into a verification failure by the binding store. Three layers disagreed about what a storable identity field is. The configuration schema and the probe parser both accepted control characters, while the durable binding store rejected any code point below 0x20 or equal to 0x7f. A fingerprint that genuinely matched its probe therefore returned "verified" and was immediately downgraded to failed / IDENTITY_BINDING_ UNAVAILABLE, because the store raises its record rejection outside save()'s own error handling, the manager catches it as a bare failure, and the resulting unavailable flag is process-sticky and so downgraded identity verification for every profile in that process. Extract the single shared predicate and use it in all three layers so they cannot drift again. An unstorable fingerprint is now refused at configuration time with its exact path, and unstorable probe evidence can never reach the store. Doctor appends the IDENTITY_* code to its explanation so the cause is recoverable from its output rather than requiring a patched build. Fail-closed behavior on a genuine binding-storage outage is deliberate and is unchanged. Probe tools whose response spans multiple lines remain unusable for identity verification; that limitation is now reported by validate instead of surfacing as an unexplained runtime failure. Refs #451 Co-authored-by: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +++ src/cli/doctor.ts | 4 ++- src/config/schema.ts | 10 ++++++- src/identity/identity-binding-store.ts | 6 ++--- src/identity/identity-manager.ts | 4 ++- src/utils/control-characters.ts | 12 +++++++++ tests/config.test.ts | 24 +++++++++++++++++ tests/doctor.test.ts | 8 +++--- tests/identity-manager.test.ts | 36 ++++++++++++++++++++++++++ 9 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 src/utils/control-characters.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 64d99df5..99f121f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format ## [Unreleased] +### Fixed + +- [#451](https://github.com/mohanagy/miftah/issues/451) Stopped a configured identity fingerprint containing a control character from validating, verifying, and then being silently converted into a verification failure. Three layers disagreed about what a storable identity field is: the configuration schema and the probe parser both accepted control characters, while the durable binding store rejected any code point below `0x20` or equal to `0x7f`. A fingerprint that genuinely matched its probe therefore returned `verified` and was then rewritten to `failed` / `IDENTITY_BINDING_UNAVAILABLE`, because the store's record rejection is raised outside `save()`'s own error handling, is caught as a bare failure, and sets a process-sticky unavailable flag that downgraded identity verification for every profile in that process. The single shared predicate is now used by all three layers, so an unstorable fingerprint is refused at configuration time with its exact path and unstorable probe evidence can never reach the store. `doctor` now appends the `IDENTITY_*` code to its explanation, so the cause is recoverable from its output rather than requiring a patched build. Fail-closed behavior on a genuine binding-storage outage is unchanged. Probe tools whose response spans multiple lines remain unusable for identity verification; that limitation is now reported by `miftah validate` instead of surfacing as an unexplained runtime failure. + ## [1.1.5] - 2026-09-11 ### Changed diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index d1c70280..df9a23c1 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -686,7 +686,9 @@ export async function runDoctor(configPath: string): Promise { : identityCheck( required ? "error" : "warning", targetText, - "Configured upstream identity verification did not complete.", + `Configured upstream identity verification did not complete${ + identity.errorCode === undefined ? "" : ` (${identity.errorCode})` + }.`, identity.status === "unsupported" ? "Upgrade or configure the upstream to expose the expected read-only, no-required-input identity probe. Property access is not account identity evidence." : "Review the configured expected fingerprint and identity probe before relying on risky operations." diff --git a/src/config/schema.ts b/src/config/schema.ts index b117d909..8d1d1fb3 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -4,6 +4,7 @@ import { canonicalizeOAuthResource } from "../oauth/canonical-resource.js"; import { parseOAuthConnectionRef, validateOAuthIssuer } from "../oauth/connection-types.js"; import { isSafeOAuthHttpsUrl } from "../oauth/url-safety.js"; import { hasMergedHeader } from "../upstream/headers.js"; +import { containsControlCharacter } from "../utils/control-characters.js"; import { SUPPORTED_CONFIG_VERSIONS } from "./versions.js"; const recordSchema = z.record(z.string(), z.unknown()); @@ -153,7 +154,14 @@ function isLoopbackHostname(hostname: string): boolean { ); } -const identityFieldSchema = z.string().trim().min(1).max(256); +const identityFieldSchema = z + .string() + .trim() + .min(1) + .max(256) + .refine((value) => !containsControlCharacter(value), { + message: "identity fields cannot contain control characters" + }); const opaqueIdentityAccountIdSchema = z .string() .regex(/^[A-Za-z0-9](?:[A-Za-z0-9._:-]{0,255})$/u, { diff --git a/src/identity/identity-binding-store.ts b/src/identity/identity-binding-store.ts index 58583e02..11614750 100644 --- a/src/identity/identity-binding-store.ts +++ b/src/identity/identity-binding-store.ts @@ -3,6 +3,7 @@ import { chmod, mkdir, open, readFile, rename, rm } from "node:fs/promises"; import { homedir, platform } from "node:os"; import { basename, dirname, isAbsolute, join, resolve } from "node:path"; import { OAuthLocalLockUnavailableError, withOAuthLocalLock } from "../oauth/local-lock.js"; +import { containsControlCharacter } from "../utils/control-characters.js"; import { MiftahError } from "../utils/errors.js"; import type { IdentityBindingRecord, IdentityBindingStore } from "./identity-manager.js"; @@ -36,10 +37,7 @@ function boundedIdentifier(value: unknown): value is string { value.length > 0 && value.length <= 256 && value.trim() === value && - ![...value].some((character) => { - const code = character.codePointAt(0); - return code === undefined || code < 0x20 || code === 0x7f; - }) + !containsControlCharacter(value) ); } diff --git a/src/identity/identity-manager.ts b/src/identity/identity-manager.ts index d0ee62de..31b59592 100644 --- a/src/identity/identity-manager.ts +++ b/src/identity/identity-manager.ts @@ -3,6 +3,7 @@ import type { Tool } from "@modelcontextprotocol/server"; import type { IdentityConfig, IdentityFingerprint, MiftahConfig, RiskLevel, ToolingConfig } from "../config/types.js"; import { classifyRisk } from "../policy/risk-classifier.js"; import type { UpstreamRequestOptions, UpstreamSession } from "../upstream/upstream-session.js"; +import { containsControlCharacter } from "../utils/control-characters.js"; import { MiftahError } from "../utils/errors.js"; import type { IdentityProbeCapabilityDiagnostic, IdentityStatus } from "./identity-types.js"; @@ -591,7 +592,8 @@ function matches(expected: IdentityFingerprint, actual: IdentityFingerprint): bo function boundedIdentityField(value: string): string | undefined { const normalized = value.trim(); - return normalized.length > 0 && normalized.length <= maxIdentityFieldLength ? normalized : undefined; + if (normalized.length === 0 || normalized.length > maxIdentityFieldLength) return undefined; + return containsControlCharacter(normalized) ? undefined : normalized; } function boundedFingerprintField(field: keyof IdentityFingerprint, value: string): string | undefined { diff --git a/src/utils/control-characters.ts b/src/utils/control-characters.ts new file mode 100644 index 00000000..7aea259a --- /dev/null +++ b/src/utils/control-characters.ts @@ -0,0 +1,12 @@ +/** + * Identity fingerprints travel from configuration through verification into the durable + * binding store, and every layer must agree on what a storable field looks like. The + * binding store cannot persist C0 control characters or DEL, so they are rejected at the + * configuration boundary rather than surfacing later as an unavailable binding store. + */ +export function containsControlCharacter(value: string): boolean { + return [...value].some((character) => { + const code = character.codePointAt(0); + return code === undefined || code < 0x20 || code === 0x7f; + }); +} diff --git a/tests/config.test.ts b/tests/config.test.ts index ccf92417..4fee20e4 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -649,6 +649,30 @@ describe("config foundation", () => { ).toThrow(duplicateIdentityRiskPattern); }); + it.each([ + ["a newline", "mona\nextra"], + ["a tab", "mona\textra"], + ["a delete character", "mona\u007fextra"] + ])("rejects an identity fingerprint containing %s", (_label, login) => { + expect(() => + validateConfig({ + version: "1", + name: "github", + defaultProfile: "work", + upstream: { transport: "stdio", command: "node", args: ["server.js"] }, + profiles: { + work: { + identity: { + expected: { login }, + probe: { tool: "whoami", resultFormat: "text" }, + maxAgeMs: 60_000 + } + } + } + }) + ).toThrow(identityExpectedLoginPattern); + }); + it.each([ ["accountId", { accountId: "google-sub-work" }], ["organization", { organization: "github" }], diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 00b0e928..96efe0a2 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -287,7 +287,7 @@ describe("doctor readiness runner", () => { expect(check(report, DOCTOR_CODES.TOOLS_DISCOVERY).status).toBe("pass"); expect(check(report, DOCTOR_CODES.IDENTITY)).toMatchObject({ status: "error", - explanation: "Configured upstream identity verification did not complete." + explanation: "Configured upstream identity verification did not complete (IDENTITY_MISMATCH)." }); expect(report).toMatchObject({ overallStatus: "failed", ok: false }); expect(codes.indexOf(DOCTOR_CODES.TOOLS_DISCOVERY)).toBeLessThan(codes.indexOf(DOCTOR_CODES.IDENTITY)); @@ -327,7 +327,7 @@ describe("doctor readiness runner", () => { expect(check(report, DOCTOR_CODES.TOOLS_DISCOVERY).status).toBe("pass"); expect(check(report, DOCTOR_CODES.IDENTITY)).toMatchObject({ status: "warning", - explanation: "Configured upstream identity verification did not complete." + explanation: "Configured upstream identity verification did not complete (IDENTITY_VERIFICATION_FAILED)." }); expect(report).toMatchObject({ overallStatus: "degraded", ok: true }); for (const sensitiveValue of [configPath, rawIdentityResponse, expectedAccount, "whoami", "miftah_verify_identity"]) { @@ -357,7 +357,7 @@ describe("doctor readiness runner", () => { expect(check(report, DOCTOR_CODES.TOOLS_DISCOVERY).status).toBe("pass"); expect(check(report, DOCTOR_CODES.IDENTITY)).toMatchObject({ status: "warning", - explanation: "Configured upstream identity verification did not complete.", + explanation: "Configured upstream identity verification did not complete (IDENTITY_PROBE_UNSUPPORTED).", remediation: "Upgrade or configure the upstream to expose the expected read-only, no-required-input identity probe. Property access is not account identity evidence." }); expect(report).toMatchObject({ overallStatus: "degraded", ok: true }); @@ -388,7 +388,7 @@ describe("doctor readiness runner", () => { expect(check(report, DOCTOR_CODES.PROMPTS_DISCOVERY).status).toBe("pass"); expect(check(report, DOCTOR_CODES.IDENTITY)).toMatchObject({ status: "warning", - explanation: "Configured upstream identity verification did not complete." + explanation: "Configured upstream identity verification did not complete (IDENTITY_PROBE_UNSUPPORTED)." }); expect(report).toMatchObject({ overallStatus: "degraded", ok: true }); }); diff --git a/tests/identity-manager.test.ts b/tests/identity-manager.test.ts index 3e7f614f..aa7520b7 100644 --- a/tests/identity-manager.test.ts +++ b/tests/identity-manager.test.ts @@ -1117,6 +1117,42 @@ describe("identity verifier", () => { }); }); + it("refuses probe evidence containing control characters instead of binding unstorable evidence", async () => { + const config = validateConfig({ + version: "1", + name: "identity-test", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { + env: { TEST_ACCOUNT_NAME: "mona\nextra" }, + identity: { + expected: { login: "mona" }, + probe: { tool: "whoami", resultFormat: "text" }, + maxAgeMs: 60_000 + } + } + } + }); + const upstreams = new UpstreamProcessManager(config.upstream!, config.profiles); + managers.push(upstreams); + let saves = 0; + const verifier = new IdentityManager(config, { + bindingStore: { + load: async () => [], + save: async () => { + saves += 1; + } + } + }); + + const result = await verifier.verify("work", undefined, await upstreams.get("work")); + + expect(result).toMatchObject({ status: "failed", errorCode: "IDENTITY_VERIFICATION_FAILED" }); + expect(saves).toBe(0); + expect(verifier.status("work", undefined)).not.toMatchObject({ errorCode: "IDENTITY_BINDING_UNAVAILABLE" }); + }); + it("reports persisted evidence as expired without using it as a live result", async () => { const config = validateConfig({ version: "1", From 5ebba3405bba7264b08d7a34e51da1c8541ee4bd Mon Sep 17 00:00:00 2001 From: Mohammed Naji Date: Fri, 18 Sep 2026 01:47:38 +0400 Subject: [PATCH 2/2] chore(release): prepare v1.1.6 (#454) Finalize the compatible v1.1.6 patch release delivering the identity fingerprint fix from #451. No public API, dependency, credential, routing, redaction, or audit behavior changed, and fail-closed behavior on a genuine binding-storage outage is unchanged. Two boundaries tighten deliberately. A configuration carrying a control character in an identity fingerprint is now rejected at validation with its exact path, and probe output containing control characters is no longer accepted as an identity field. Neither could previously produce a durable binding, so no working configuration is invalidated. Publication remains gated on exact development-to-main promotion and protected OIDC trusted publishing, registry provenance, a fresh install, and package signature verification; this release does not authorize removal of any legacy behavior. Refs #453 Co-authored-by: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++++++ README.md | 2 +- docs/mcp-compatibility.md | 2 +- docs/presets-and-clients.md | 2 +- docs/whats-new-in-0.5.md | 4 ++-- package-lock.json | 4 ++-- package.json | 2 +- tests/release-version.test.ts | 19 ++++++++++--------- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99f121f5..0bbeffc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,16 @@ All notable changes to this project will be documented in this file. The format ## [Unreleased] +## [1.1.6] - 2026-09-18 + ### Fixed - [#451](https://github.com/mohanagy/miftah/issues/451) Stopped a configured identity fingerprint containing a control character from validating, verifying, and then being silently converted into a verification failure. Three layers disagreed about what a storable identity field is: the configuration schema and the probe parser both accepted control characters, while the durable binding store rejected any code point below `0x20` or equal to `0x7f`. A fingerprint that genuinely matched its probe therefore returned `verified` and was then rewritten to `failed` / `IDENTITY_BINDING_UNAVAILABLE`, because the store's record rejection is raised outside `save()`'s own error handling, is caught as a bare failure, and sets a process-sticky unavailable flag that downgraded identity verification for every profile in that process. The single shared predicate is now used by all three layers, so an unstorable fingerprint is refused at configuration time with its exact path and unstorable probe evidence can never reach the store. `doctor` now appends the `IDENTITY_*` code to its explanation, so the cause is recoverable from its output rather than requiring a patched build. Fail-closed behavior on a genuine binding-storage outage is unchanged. Probe tools whose response spans multiple lines remain unusable for identity verification; that limitation is now reported by `miftah validate` instead of surfacing as an unexplained runtime failure. +### Changed + +- [#453](https://github.com/mohanagy/miftah/issues/453) Prepared the compatible v1.1.6 patch release delivering the identity-fingerprint fix. No public API, dependency, credential, routing, redaction, or audit behavior changed, and fail-closed behavior on a genuine binding-storage outage is unchanged. Two boundaries tighten deliberately: a configuration carrying a control character in an identity fingerprint is now rejected at validation with its exact path, and probe output containing control characters is no longer accepted as an identity field. Neither could previously produce a durable binding, so no working configuration is invalidated. Publication remains gated on exact `development`-to-`main` promotion and protected OIDC trusted publishing, registry provenance, a fresh install, and package-signature verification; this release does not authorize removal of any legacy behavior. + ## [1.1.5] - 2026-09-11 ### Changed diff --git a/README.md b/README.md index 29f0d423..a8d74f07 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ Install Miftah, then choose the terminal wizard or the browser Console. Both use ### 1. Install the current release ```bash -npm install -g @lubab/miftah@1.1.5 +npm install -g @lubab/miftah@1.1.6 miftah version ``` diff --git a/docs/mcp-compatibility.md b/docs/mcp-compatibility.md index 6d7f7c0b..70869a31 100644 --- a/docs/mcp-compatibility.md +++ b/docs/mcp-compatibility.md @@ -2,7 +2,7 @@ This page is the compatibility source of truth for Miftah's downstream MCP server. It records protocol-era behavior separately from generated client-configuration support and from upstream MCP transport support. A generated snippet proves only that Miftah emitted the documented JSON shape; it does not prove that an untested host completed a protocol exchange. -- Miftah baseline: `1.1.5` +- Miftah baseline: `1.1.6` - Locked MCP TypeScript packages: `@modelcontextprotocol/client`, `core`, `server`, `node`, and `server-legacy` `2.0.0` - Evidence date: 2026-08-23 - Modern protocol era: `2026-07-28` diff --git a/docs/presets-and-clients.md b/docs/presets-and-clients.md index ebba8d79..1e349af7 100644 --- a/docs/presets-and-clients.md +++ b/docs/presets-and-clients.md @@ -5,7 +5,7 @@ This is the compatibility source of truth for generated `miftah init` configurat For downstream protocol eras and real packaged-host evidence, see [MCP protocol and client compatibility](mcp-compatibility.md). The tables below validate generated configuration shapes; they do not by themselves establish a runtime exchange with Claude Desktop, Claude Code, Cursor, or VS Code. - Catalog version: `3` -- Miftah package version: `1.1.5` +- Miftah package version: `1.1.6` - Last tested / validation boundary: the catalog builds strict Miftah configuration that `validateConfig` accepts. The docs contract test checks generated configuration only; it does **not** construct a runtime, start, authenticate to, or smoke-test external providers. Miftah itself requires Node.js `>=20`. That does not establish an upstream server's Node requirement. diff --git a/docs/whats-new-in-0.5.md b/docs/whats-new-in-0.5.md index 6385b09f..59b4ba95 100644 --- a/docs/whats-new-in-0.5.md +++ b/docs/whats-new-in-0.5.md @@ -1,9 +1,9 @@ # What is in Miftah 0.5 -Install `@lubab/miftah@1.1.5`, the current stable release, to use the guided setup and account-management capabilities introduced in 0.5 instead of assembling a multi-account configuration by hand: +Install `@lubab/miftah@1.1.6`, the current stable release, to use the guided setup and account-management capabilities introduced in 0.5 instead of assembling a multi-account configuration by hand: ```bash -npm install -g @lubab/miftah@1.1.5 +npm install -g @lubab/miftah@1.1.6 miftah version ``` diff --git a/package-lock.json b/package-lock.json index 5c9ec00b..ba6a4001 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@lubab/miftah", - "version": "1.1.5", + "version": "1.1.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@lubab/miftah", - "version": "1.1.5", + "version": "1.1.6", "license": "MIT", "dependencies": { "@hono/node-server": "2.1.1", diff --git a/package.json b/package.json index 4c32b8ff..c892be97 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lubab/miftah", - "version": "1.1.5", + "version": "1.1.6", "description": "Wrap any MCP. Use the right account without reconnecting.", "keywords": [ "mcp", diff --git a/tests/release-version.test.ts b/tests/release-version.test.ts index d72ca83f..b7794778 100644 --- a/tests/release-version.test.ts +++ b/tests/release-version.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; -const releaseVersion = "1.1.5"; +const releaseVersion = "1.1.6"; function readRepositoryFile(path: string): string { return readFileSync(new URL(`../${path}`, import.meta.url), "utf8"); @@ -21,15 +21,15 @@ function releaseNotes(changelog: string, version: string): string { return changelog.slice(match.index, end < 0 ? undefined : end); } -describe("v1.1.5 release artifacts", () => { +describe("v1.1.6 release artifacts", () => { it.each([ { name: "a non-zero-padded date", - changelog: "## [1.1.5] - 2026-9-11\n\n### Changed\n" + changelog: "## [1.1.6] - 2026-9-18\n\n### Changed\n" }, { name: "a heading that does not start its line", - changelog: "Release candidate: ## [1.1.5] - 2026-09-11\n\n### Changed\n" + changelog: "Release candidate: ## [1.1.6] - 2026-09-18\n\n### Changed\n" } ])("rejects $name", ({ changelog }) => { expect(() => releaseNotes(changelog, releaseVersion)).toThrow( @@ -71,17 +71,18 @@ describe("v1.1.5 release artifacts", () => { } }); - it("documents the v1.1.5 dependency refresh and its published boundary", () => { + it("documents the v1.1.6 identity fingerprint fix and its published boundary", () => { const changelog = readRepositoryFile("CHANGELOG.md"); const notes = releaseNotes(changelog, releaseVersion); + expect(notes).toContain("### Fixed"); expect(notes).toContain("### Changed"); - for (const issue of [446, 447]) { + for (const issue of [451, 453]) { expect(notes).toContain(`[#${issue}](https://github.com/mohanagy/miftah/issues/${issue})`); } - expect(notes).toContain("@hono/node-server"); - expect(notes).toContain("dependency refresh rather than a security release"); - expect(notes).toContain("This toolchain work is not published"); + expect(notes).toContain("control character"); + expect(notes).toContain("refused at configuration time with its exact path"); + expect(notes).toContain("fail-closed behavior on a genuine binding-storage outage is unchanged"); expect(notes).toContain("protected OIDC trusted publishing"); expect(notes).toContain("registry provenance"); expect(notes).toContain("does not authorize removal of any legacy behavior");