[PR #6548] [CLOSED] fix(ida): exclude header and environment values from the registration digest #7378

Closed
opened 2026-11-01 21:47:25 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6548
Author: @Adolanium
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/ida-registration-digest-credentials


📝 Commits (2)

  • 5f2b3c0 fix(ida): exclude header and environment values from the registration digest
  • 0fb4ed2 fix(ida): withhold only marked credentials from the registration digest and rebind on private changes

📊 Changes

9 files changed (+225 additions, -13 deletions)

View changed files

📝 docs/ida-provider.md (+2 -2)
📝 src/application/AnalysisProvider.ts (+6 -0)
📝 src/application/binary/AnalysisProviderEvaluation.ts (+14 -3)
📝 src/application/binary/AnalysisProviderRegistry.ts (+6 -0)
📝 src/application/binary/BinarySession.ts (+5 -2)
📝 src/ida/IdaConfiguration.ts (+52 -1)
📝 src/ida/IdaProvider.test.ts (+97 -0)
📝 src/ida/IdaProvider.ts (+4 -5)
📝 tests/composition/analysis-sessions/providerBinding.test.ts (+39 -0)

📄 Description

Summary

Fixes #6549.

Status: draft until a maintainer approves the approach. It adds an optional field to the IDA registration and a provider-private binding key to shared session reuse. See the review threads.

The IDA analysis profile records a registration_digest, the SHA-256 of the whole MCP registration. The profile is returned with analysis results and retained in Evidence, so the digest committed to credential values in headers or env: anyone holding a candidate token could check it against the digest. This PR lets a registration mark which headers / env entries are secret, withholds only those values from the public digest, and keeps the full registration as a private binding key. The session compares that key before it reuses a live client. A rotated credential therefore reconnects without entering the profile, and non-secret values such as IDADIR keep distinguishing profiles.

Problem and expected behavior

IdaProvider.resolveAnalysisProfile set registration_digest to a hash of the full parsed registration. Two constraints apply:

  • AGENTS.md asks REA to redact transport authentication credentials and explicitly marked sensitive values, without inferring secrecy from names. So which values to withhold has to be stated in the registration.
  • The analysis profile is also what BinarySession compares before it reuses a live client for the same target. A digest without secret values would make a rotated token look unchanged and keep the old connection. An earlier revision of this PR did exactly that, and review caught it.

Expected: marked values never reach the profile, every other registration value still distinguishes profiles, and a change to any value, marked or not, starts a new client.

Change and scope

  • src/ida/IdaConfiguration.ts: the registration accepts an optional sensitive list of names. Each name must be an entry of that registration's headers (HTTP) or env (stdio), or the registration is invalid. idaRegistrationDigest replaces only those values with [REDACTED], and keeps every other value, all names and the sensitive list itself. idaRegistrationBindingKey hashes the complete registration.
  • src/application/AnalysisProvider.ts: AnalysisProfileResolution gains an optional, never-serialized bindingKey next to the existing provider-private createClient. The candidate evaluation and the registry binding carry it the same way they carry createClient.
  • src/application/binary/BinarySession.ts: the same-target reuse check also requires an equal bindingKey. Providers that return none behave as before.
  • IdaProvider.resolveAnalysisProfile returns the binding key with the profile.
  • docs/ida-provider.md documents sensitive and what the digest covers.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contribution scope: this extends the IDA registration contract (new optional sensitive field) and the shared provider-binding contract (bindingKey). It is left as a draft for a maintainer decision on that approach; see the review threads.

Contract and boundary impact

  • Semantic owner and earliest changed stage: IDA registration parsing and identity in IdaConfiguration.ts; client reuse in BinarySession.
  • Contribution scope fit and linked maintainer decision: pending, see the review threads.
  • CLI and MCP/tool-catalog contract: no tool schema change. IDA registrations may add sensitive; registrations without it keep today's digest. A registration that lists an unknown name fails validation.
  • Provider, bridge, target-format, or platform compatibility: bindingKey is optional; only IDA returns one.
  • Evidence, artifact, provenance, or reconstruction contract: marked values are withheld from the profile and Evidence. The binding key is never serialized: status output, snapshots and Evidence carry only the profile.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): a changed registration value, including a marked one, now starts a new client for the same target and closes the old one.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:
    • src/ida/IdaProvider.test.ts, "withholds only marked HTTP header values from the public digest": with sensitive: ["Authorization"], a rotated bearer token keeps the digest but changes the binding key; a changed unmarked X-Backend header changes both; the same registration without sensitive gives a different digest.
    • "withholds only marked stdio environment values from the public digest": a rotated IDA_API_KEY keeps the digest and changes the binding key; a different IDADIR changes the digest.
    • "rejects a sensitive name that the registration does not define".
    • tests/composition/analysis-sessions/providerBinding.test.ts, "starts a new client when only the provider-private binding key changes": reopening and executing with an equal profile and key reuses the client; changing only the key creates a new client and closes the old one; session status never contains the key.
  • Base reproduction or other evidence: negative checks at 0fb4ed21d. Dropping the key from the reuse check fails the session test. Withholding every value fails both IDA digest tests. Using the public digest as the binding key also fails both IDA digest tests.
  • User-visible CLI/MCP output (if applicable): registration_digest values in IDA analysis profiles for registrations that use sensitive.
  • Remaining proof gaps: no run against a real IDA MCP server.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

On Linux (WSL Ubuntu, Node 24.22.1) at 0fb4ed21d (on upstream/main 6c4d880f2), after npm run build:cached:

  • npx tsc -p . --noEmit passed.
  • npx oxfmt --check on the changed files: clean. npx oxlint on the eight changed source and test files: 16 warnings, the same as on main.
  • npm run docs:check passed.
  • npx vitest run src/ida tests/boundary/ida tests/composition/idaProvider.test.ts tests/composition/analysis-sessions src/application/binary: 45 files, 634 tests passed, in two runs.
  • The three negative checks above failed their tests.

Compatibility, safety, and release

  • Breaking changes or migration steps: none required. Users who keep credentials in headers or env should list them in sensitive.
  • Real Hopper/Ghidra, browser, or OS coverage: Linux only; no real IDA.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: marked credentials leave the profile and Evidence; the full registration is compared only in memory.

Author readiness

Human owner: @Adolanium

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6548 **Author:** [@Adolanium](https://github.com/Adolanium) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/ida-registration-digest-credentials` --- ### 📝 Commits (2) - [`5f2b3c0`](https://github.com/morluto/rea/commit/5f2b3c05012590690a1ae2616b0f83fa4a6a59aa) fix(ida): exclude header and environment values from the registration digest - [`0fb4ed2`](https://github.com/morluto/rea/commit/0fb4ed21d1568aa4bfadc5f514907a9497ecee88) fix(ida): withhold only marked credentials from the registration digest and rebind on private changes ### 📊 Changes **9 files changed** (+225 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `docs/ida-provider.md` (+2 -2) 📝 `src/application/AnalysisProvider.ts` (+6 -0) 📝 `src/application/binary/AnalysisProviderEvaluation.ts` (+14 -3) 📝 `src/application/binary/AnalysisProviderRegistry.ts` (+6 -0) 📝 `src/application/binary/BinarySession.ts` (+5 -2) 📝 `src/ida/IdaConfiguration.ts` (+52 -1) 📝 `src/ida/IdaProvider.test.ts` (+97 -0) 📝 `src/ida/IdaProvider.ts` (+4 -5) 📝 `tests/composition/analysis-sessions/providerBinding.test.ts` (+39 -0) </details> ### 📄 Description ## Summary Fixes #6549. > **Status: draft until a maintainer approves the approach.** It adds an optional field to the IDA registration and a provider-private binding key to shared session reuse. See the review threads. The IDA analysis profile records a `registration_digest`, the SHA-256 of the whole MCP registration. The profile is returned with analysis results and retained in Evidence, so the digest committed to credential values in `headers` or `env`: anyone holding a candidate token could check it against the digest. This PR lets a registration mark which `headers` / `env` entries are secret, withholds only those values from the public digest, and keeps the full registration as a private binding key. The session compares that key before it reuses a live client. A rotated credential therefore reconnects without entering the profile, and non-secret values such as `IDADIR` keep distinguishing profiles. ## Problem and expected behavior `IdaProvider.resolveAnalysisProfile` set `registration_digest` to a hash of the full parsed registration. Two constraints apply: - AGENTS.md asks REA to redact transport authentication credentials and explicitly marked sensitive values, without inferring secrecy from names. So which values to withhold has to be stated in the registration. - The analysis profile is also what `BinarySession` compares before it reuses a live client for the same target. A digest without secret values would make a rotated token look unchanged and keep the old connection. An earlier revision of this PR did exactly that, and review caught it. Expected: marked values never reach the profile, every other registration value still distinguishes profiles, and a change to any value, marked or not, starts a new client. ## Change and scope - `src/ida/IdaConfiguration.ts`: the registration accepts an optional `sensitive` list of names. Each name must be an entry of that registration's `headers` (HTTP) or `env` (stdio), or the registration is invalid. `idaRegistrationDigest` replaces only those values with `[REDACTED]`, and keeps every other value, all names and the `sensitive` list itself. `idaRegistrationBindingKey` hashes the complete registration. - `src/application/AnalysisProvider.ts`: `AnalysisProfileResolution` gains an optional, never-serialized `bindingKey` next to the existing provider-private `createClient`. The candidate evaluation and the registry binding carry it the same way they carry `createClient`. - `src/application/binary/BinarySession.ts`: the same-target reuse check also requires an equal `bindingKey`. Providers that return none behave as before. - `IdaProvider.resolveAnalysisProfile` returns the binding key with the profile. - `docs/ida-provider.md` documents `sensitive` and what the digest covers. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. Contribution scope: this extends the IDA registration contract (new optional `sensitive` field) and the shared provider-binding contract (`bindingKey`). It is left as a draft for a maintainer decision on that approach; see the review threads. ## Contract and boundary impact - Semantic owner and earliest changed stage: IDA registration parsing and identity in `IdaConfiguration.ts`; client reuse in `BinarySession`. - Contribution scope fit and linked maintainer decision: pending, see the review threads. - CLI and MCP/tool-catalog contract: no tool schema change. IDA registrations may add `sensitive`; registrations without it keep today's digest. A registration that lists an unknown name fails validation. - Provider, bridge, target-format, or platform compatibility: `bindingKey` is optional; only IDA returns one. - Evidence, artifact, provenance, or reconstruction contract: marked values are withheld from the profile and Evidence. The binding key is never serialized: status output, snapshots and Evidence carry only the profile. - Target selection, authority, upstream behavior, and cleanup details (if applicable): a changed registration value, including a marked one, now starts a new client for the same target and closes the old one. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage <!-- State whether evidence is executed, source-derived, or proposed. For fixes, show the original failure and explain how the regression would fail on the affected base revision when practical. Include the most useful proof inline; link long logs or artifacts, or put them in optional details. Documentation-only changes can use source, link, or rendered-document checks. --> - Tests added or updated: - `src/ida/IdaProvider.test.ts`, "withholds only marked HTTP header values from the public digest": with `sensitive: ["Authorization"]`, a rotated bearer token keeps the digest but changes the binding key; a changed unmarked `X-Backend` header changes both; the same registration without `sensitive` gives a different digest. - "withholds only marked stdio environment values from the public digest": a rotated `IDA_API_KEY` keeps the digest and changes the binding key; a different `IDADIR` changes the digest. - "rejects a sensitive name that the registration does not define". - `tests/composition/analysis-sessions/providerBinding.test.ts`, "starts a new client when only the provider-private binding key changes": reopening and executing with an equal profile and key reuses the client; changing only the key creates a new client and closes the old one; session status never contains the key. - Base reproduction or other evidence: negative checks at 0fb4ed21d. Dropping the key from the reuse check fails the session test. Withholding every value fails both IDA digest tests. Using the public digest as the binding key also fails both IDA digest tests. - User-visible CLI/MCP output (if applicable): `registration_digest` values in IDA analysis profiles for registrations that use `sensitive`. - Remaining proof gaps: no run against a real IDA MCP server. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed On Linux (WSL Ubuntu, Node 24.22.1) at 0fb4ed21d (on upstream/main 6c4d880f2), after `npm run build:cached`: - `npx tsc -p . --noEmit` passed. - `npx oxfmt --check` on the changed files: clean. `npx oxlint` on the eight changed source and test files: 16 warnings, the same as on main. - `npm run docs:check` passed. - `npx vitest run src/ida tests/boundary/ida tests/composition/idaProvider.test.ts tests/composition/analysis-sessions src/application/binary`: 45 files, 634 tests passed, in two runs. - The three negative checks above failed their tests. ## Compatibility, safety, and release - Breaking changes or migration steps: none required. Users who keep credentials in `headers` or `env` should list them in `sensitive`. - Real Hopper/Ghidra, browser, or OS coverage: Linux only; no real IDA. - Package or release metadata impact: none. - Security, privacy, process, or containment review: marked credentials leave the profile and Evidence; the full registration is compared only in memory. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @Adolanium - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:47:25 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7378
No description provided.