fix(identity): reject control characters in identity fingerprints

[mohanagy]

Pull request

Summary

Fixes #451. 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. The operator saw only Configured upstream identity verification did not complete. with no recoverable cause.

Three layers disagreed about what a storable identity field is:

Layer	Control characters
identityFieldSchema (src/config/schema.ts)	accepted
boundedIdentityField (src/identity/identity-manager.ts)	accepted — probe matched, verifier returned verified
boundedIdentifier (src/identity/identity-binding-store.ts)	rejected (< 0x20 or 0x7f)

The rejection was masked three times: normalizeRecords() raises outside save()'s own try, the manager catches it bare and sets the process-sticky bindingStoreUnavailable, and applyBindingResult() then rewrote the already-verified status to failed / IDENTITY_BINDING_UNAVAILABLE. Because the flag is per-process, one bad fingerprint downgraded identity verification for every profile in that process.

This extracts the single shared predicate (src/utils/control-characters.ts) and uses it in all three layers so they cannot drift again, and has doctor append the IDENTITY_* code to its explanation.

Observed before the fix, from one instrumented run: MATCHED= true → RETURNING verified → doctor received status= failed errorCode= IDENTITY_BINDING_UNAVAILABLE.

Security impact

Fail-closed behavior on a genuine binding-storage outage is deliberate and is unchanged — tests/identity-manager.test.ts still asserts status: "failed", bindingState: "unavailable", errorCode: "IDENTITY_BINDING_UNAVAILABLE" when save() throws. No credential handling, routing, redaction, audit, subprocess, or dependency change.

Two boundaries move deliberately, both tightening:

• Configuration: a fingerprint with a control character is now rejected at validation with its exact path, instead of being accepted and failing later. This can reject a configuration that previously validated — but any such configuration could never have produced a durable binding, so it was already non-functional.
• Probe evidence: probe output containing control characters is no longer treated as a usable identity field, so unstorable evidence cannot reach the store or trip the sticky flag.

The IDENTITY_* codes added to doctor output are fixed enumeration members, not identity values; tests/doctor.test.ts continues to assert the report contains no config path, account name, probe tool name, or management-tool name.

Probe tools whose response spans multiple lines remain unusable for identity verification. That limitation is now reported by miftah validate rather than surfacing as an unexplained runtime failure. Making them usable needs field extraction in the probe rather than whole-response matching, which is a feature and out of scope.

Validation

• A failing test was observed first for each behavior or configuration-contract change — 4 failing assertions before the fix (3 new schema cases, 1 new probe-evidence case), then green.
• npm run lint — 728 problems, all pre-existing .worktrees tsconfigRootDir parser errors. Baseline on a clean tree is 727; the +1 is the new file hitting the same error. The five changed files lint clean when invoked directly.
• npm run typecheck — clean.
• npm test — 162 files, 2003 passed, 34 skipped, 0 failed.
• npm run build — success.
• node dist/cli/main.js schema — success.
• npm run check:pack — Package contract verified (58 files).
• Fixtures, logs, screenshots, and examples contain no credentials or private data. The shared evidence fixture tests/fixtures/fake-upstream-runtime.mjs is not modified, so the named-host evidence hash is intact.
• User-facing documentation and CHANGELOG.md are updated when applicable — CHANGELOG.md under [Unreleased].
• Dependency and packaged-file changes are intentional and reviewed — one new source file, no dependency change.
• Undisclosed vulnerabilities are reported privately instead of in this pull request.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Identity fingerprints and related fields containing control characters are now rejected during configuration and verification.
  • Invalid probe evidence safely fails verification without incorrectly reporting binding-store unavailability.
  • miftah validate now reports multi-line probe responses.
  • doctor provides more specific identity verification failure codes when available.

• Validation

  • Identity configuration errors now identify affected fields and reject newline, tab, and delete-control characters.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e9735ee7-7e5f-4105-82d6-cf9af485169d

📥 Commits

Reviewing files that changed from the base of the PR and between 3c6298b and b6ed989.

📒 Files selected for processing (9)

• CHANGELOG.md
• src/cli/doctor.ts
• src/config/schema.ts
• src/identity/identity-binding-store.ts
• src/identity/identity-manager.ts
• src/utils/control-characters.ts
• tests/config.test.ts
• tests/doctor.test.ts
• tests/identity-manager.test.ts

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

📝 Walkthrough

Walkthrough

Identity fingerprints now reject control characters during configuration and probe parsing. A shared predicate supports all relevant validation paths. Doctor reports identity error codes, and tests cover failed verification without binding-store unavailability.

Changes

Identity fingerprint validation

Layer / File(s)	Summary
Shared control-character validation src/utils/control-characters.ts, src/config/schema.ts, src/identity/identity-manager.ts, src/identity/identity-binding-store.ts	A shared predicate rejects control characters in configured fields, probe evidence, and binding identifiers.
Verification regression coverage tests/config.test.ts, tests/identity-manager.test.ts	Tests cover newline, tab, and delete characters. Failed verification does not save a binding or report binding unavailability.
Diagnostic reporting and release notes src/cli/doctor.ts, tests/doctor.test.ts, CHANGELOG.md	Doctor includes identity error codes. Tests verify the diagnostic suffixes, and the changelog records the fixes.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to b6ed9

No concrete merge-blocking risk remains in the current change.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Docstring Coverage	❌ Error	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 8 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting control characters in identity fingerprints.
Description check	✅ Passed	The description includes the required Summary, Security impact, and Validation sections. It explains the failure mode, security behavior, test results, commands, documentation updates, and scope limit…
Linked Issues check	✅ Passed	The changes satisfy the coding requirements in #451. containsControlCharacter is shared by configuration validation, identity evidence validation, and binding storage. Configuration tests cover newl…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within #451. The changelog entry, shared utility, validation changes, diagnostic changes, and related tests directly support the issue objectives. No unrelated product behavi…

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 8 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{A rabbit checks each fingerprint line
Control marks stay out of the bind
Doctor names the failure code
Safe verification keeps its mode
Multi-line probes now show their sign}

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Pre-merge checks failed. Please resolve the failing checks before merging.