Conversation
Adds an optional private_manifest_url to IronHubInstallOptions so install resolves an org-scoped signed manifest instead of the public catalog, threaded through the agent install capability and a new --private-manifest-url flag. Private manifests are one-shot and uniquely tokened, so they fetch and verify directly rather than entering the public catalog's cache, single-flight, and replay-monotonic machinery. Agent half of IronHub private spaces.
The binding validator rejects legacy top-level [[capabilities]] for InstalledLocal manifest sources, breaking the extension CLI fixtures and the available_extensions filesystem-catalog fixture. Move both to the [[host_api]] ironclaw.capability_provider/v1 form, and switch the available_extensions fixture in-memory parse to parse_with_host_api_contracts so it accepts the new shape.
A private artifact is the org's own signed content, not untrusted community content, so it must not trip the unverified-install gate. Add an IronHubProvenance::Private tier (wire "private") that is never classified community-unverified, and assert the private-manifest install test installs without acknowledgement.
validate_artifact_url also accepts a URL whose host matches the configured catalog host, so private install works from the hub's own origin on any deployment. SSRF blocklist still applies.
Rebuilds the v1-only IronHub deep-link surface natively on Reborn: a public HMAC-verified POST /api/ironhub/register webhook and a bearer plus HMAC-verified install-delivery route, both reusing the existing IronHubService for catalog and install. The shared key threads from IRONHUB_AGENT_SHARED_KEY through RebornRuntimeInput into build_webui_services, and the register webhook mounts only when a key is configured. Gated webui-v2-beta; agent_link HMAC vectors and the webui_v2 descriptor contract pass.
Private installs now run the same replay guard as the public path, with a handler-contract test; the rest are structural review cleanups.
Scope deliver_install egress to the authenticated caller, reject replayed install nonces single-use, and map install kinds and errors through the facade. Add link-service negative-path, register-handler, provenance-reject, and webui_v2 handler tests, plus review cleanups: a single-source register-enabled gate, shared-key length validation, verifier and catalog host-pinning annotations, and reverting unrelated CLI doc-comment churn.
github-actions
Bot
added
size: XL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an IronHub link integration behind ChangesIronHub Link Integration
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces deep-linking integration with IronHub, enabling the registration and installation of tools and skills via webhooks. It adds the IronhubLinkService and its implementation RebornIronhubLinkService to verify signed payloads using HMAC-SHA256, exposes new endpoints in the WebUI router, and updates the CLI to support private manifest URLs. The review feedback highlights three key areas for improvement: preventing a potential integer overflow panic in timestamp_fresh when processing large user-controlled timestamps, avoiding panic risks from non-monotonic clock adjustments in duration_since by using recorded.elapsed(), and mitigating separator-collision vulnerabilities in manifest_replay_key through length-prefixed encoding.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+86
to
+89
| fn timestamp_fresh(ts: u64) -> bool { | ||
| let drift = chrono::Utc::now().timestamp() - ts as i64; | ||
| drift.abs() <= MAX_TIMESTAMP_DRIFT_SECS | ||
| } |
Contributor
There was a problem hiding this comment.
The subtraction chrono::Utc::now().timestamp() - ts as i64 can panic due to integer overflow in debug mode if ts is a very large value (such as 9223372036854775808, which casts to i64::MIN). Since ts is user-controlled input from the webhook payload, this represents a potential Denial of Service (DoS) vulnerability. Use safe conversion with i64::try_from and checked_sub to prevent panics.
fn timestamp_fresh(ts: u64) -> bool {
let Ok(ts_i64) = i64::try_from(ts) else {
return false;
};
chrono::Utc::now()
.timestamp()
.checked_sub(ts_i64)
.map(|drift| drift.abs() <= MAX_TIMESTAMP_DRIFT_SECS)
.unwrap_or(false)
}| let mut seen = SEEN_INSTALL_NONCES | ||
| .lock() | ||
| .unwrap_or_else(|poisoned| poisoned.into_inner()); | ||
| seen.retain(|_, recorded| now.duration_since(*recorded) < ttl); |
Contributor
There was a problem hiding this comment.
Using now.duration_since(*recorded) can panic if now is earlier than recorded due to non-monotonic clock behavior or VM/platform-level clock adjustments. Since recorded is an Instant, you can use the safer and more idiomatic recorded.elapsed() which internally uses saturating subtraction and avoids any panic risk.
Suggested change
| seen.retain(|_, recorded| now.duration_since(*recorded) < ttl); | |
| seen.retain(|_, recorded| recorded.elapsed() < ttl); |
Comment on lines
+651
to
+657
| fn manifest_replay_key(url: &str, manifest: &IronHubManifest) -> String { | ||
| let host = url::Url::parse(url) | ||
| .ok() | ||
| .and_then(|parsed| parsed.host_str().map(str::to_string)) | ||
| .unwrap_or_default(); | ||
| format!("{host}|{}", manifest.repo) | ||
| } |
Contributor
There was a problem hiding this comment.
Generating deterministic keys by simple concatenation with a delimiter (like {host}|{repo}) is vulnerable to separator-collision attacks if either component can contain the delimiter character. Use an injective length-prefixed encoding to ensure uniqueness and eliminate collision risks.
Suggested change
| fn manifest_replay_key(url: &str, manifest: &IronHubManifest) -> String { | |
| let host = url::Url::parse(url) | |
| .ok() | |
| .and_then(|parsed| parsed.host_str().map(str::to_string)) | |
| .unwrap_or_default(); | |
| format!("{host}|{}", manifest.repo) | |
| } | |
| fn manifest_replay_key(url: &str, manifest: &IronHubManifest) -> String { | |
| let host = url::Url::parse(url) | |
| .ok() | |
| .and_then(|parsed| parsed.host_str().map(str::to_string)) | |
| .unwrap_or_default(); | |
| format!("{}:{}:{}:{}", host.len(), host, manifest.repo.len(), manifest.repo) | |
| } |
References
- When generating deterministic identifiers or hashes from multiple string components, use an injective length-prefixed encoding rather than simple concatenation with a delimiter to eliminate the risk of separator-collision attacks or accidental collisions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn_composition/src/ironhub/capabilities.rs (1)
129-140: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove
private_manifest_urlfrom the model-callable install input
The handler forwards this field straight intoIronHubInstallOptions, which lets a model-supplied install request choose an arbitrary private manifest URL instead of coming through the signed link flow. Gate it here or keep it on the trusted link/CLI path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/ironhub/capabilities.rs` around lines 129 - 140, Remove private_manifest_url from the model-callable install input so untrusted requests cannot supply it directly. Update InstallInput in ironhub/capabilities.rs and any install-path handling around IronHubInstallOptions so only the trusted signed-link/CLI flow can populate this field, while model-invoked installs ignore or reject it.
🧹 Nitpick comments (3)
crates/ironclaw_product_workflow/src/reborn_services/ironhub_link.rs (1)
7-12: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDerive
Serializefor the wire enum.
IronhubInstallKindis part of the HTTP request contract; keeping wire enums symmetric avoids drift in contract tests, fixtures, and typed clients.As per coding guidelines, “Enums serialized over the network or persisted to DB must derive
Serialize+Deserializewith#[serde(rename_all = "snake_case")].”Proposed fix
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "snake_case")] pub enum IronhubInstallKind {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_product_workflow/src/reborn_services/ironhub_link.rs` around lines 7 - 12, `IronhubInstallKind` is missing `Serialize`, so the HTTP wire contract is asymmetric. Update the enum definition in `IronhubInstallKind` to derive `Serialize` alongside the existing `Deserialize`, keeping the same `#[serde(rename_all = "snake_case")]` attribute so request/response payloads and typed clients stay consistent.Source: Coding guidelines
crates/ironclaw_reborn_composition/src/ironhub/agent_link.rs (1)
15-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake signature verification require the validated shared-key type.
verify_signaturecurrently accepts any&str, so callers can bypass theIronhubSharedKey::newlength check. Accept&IronhubSharedKeyto make the invariant type-enforced.Proposed fix
-pub(super) fn verify_signature(shared_key: &str, payload: &str, sig_hex: &str) -> bool { +pub(super) fn verify_signature( + shared_key: &IronhubSharedKey, + payload: &str, + sig_hex: &str, +) -> bool { let Ok(expected) = hex::decode(sig_hex) else { // silent-ok: a non-hex signature is an invalid signature; reject it. return false; }; - let Ok(mut mac) = HmacSha256::new_from_slice(shared_key.as_bytes()) else { + let Ok(mut mac) = HmacSha256::new_from_slice(shared_key.as_str().as_bytes()) else {Also applies to: 60-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/ironhub/agent_link.rs` around lines 15 - 31, Update signature verification to require the validated key type instead of a raw string: change `verify_signature` (and any related call sites in the `agent_link` flow) to accept `&IronhubSharedKey`, and use `IronhubSharedKey::as_str()` internally where the string bytes are needed. This should make the `IronhubSharedKey::new` length check mandatory for all callers and prevent bypassing the invariant through plain `&str` inputs.crates/ironclaw_reborn_composition/src/runtime_input.rs (1)
255-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSoften these enablement docstrings.
Lines 255-258 and 372-379 read as if supplying
ironhub_agent_shared_keyalone mounts/api/ironhub/registerand enables the IronHub webhook flow. In practice,ironhub_register_enabled()/webui_ironhub_link_service()also require the local-runtime lifecycle ports andhost_runtime_http_egress, so the comments currently promise more than the code enforces. As per coding guidelines, "Comments that promise guarantees across layers must either be enforced by code/tests or softened to describe intent."Also applies to: 372-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_reborn_composition/src/runtime_input.rs` around lines 255 - 260, Soften the enablement docstrings in runtime_input.rs for `ironhub_agent_shared_key` and the related IronHub webhook fields so they describe intent rather than implying the key alone mounts `/api/ironhub/register` or enables the full flow. Update the comments near `ironhub_agent_shared_key`, `ironhub_register_enabled()`, and `webui_ironhub_link_service()` to mention the additional requirements like local-runtime lifecycle ports and `host_runtime_http_egress`, keeping the documentation aligned with the actual guards in code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_product_workflow/src/reborn_services.rs`:
- Around line 1317-1329: The `ironhub_deliver_install` flow only passes
`caller.user_id` into `IronhubLinkService::deliver_install`, so the signed
install target is not validated against the authenticated caller context. Update
`ironhub_deliver_install` and the `deliver_install` method on
`IronhubLinkService` to accept the full `WebUiAuthenticatedCaller` or explicit
expected `UserId`/`AgentId`, then compare the signed `uid`/`aid` payload against
the caller before consuming the nonce and reject any mismatch. Keep the auth
check fail-closed and preserve the caller’s tenant/user/agent scope throughout
the install path.
In `@crates/ironclaw_reborn_cli/src/commands/ironhub.rs`:
- Around line 93-95: The private manifest URL is currently accepted as a direct
argv option on the ironhub command, which exposes tokenized/private URLs via
shell history and process listings. Replace the `private_manifest_url` field in
the CLI parser with a non-argv input path such as a file, stdin, or env-var
indirection, and update the command handling code that consumes it so the URL is
loaded indirectly before use. Also adjust any related code paths referenced by
the `ironhub` command to stop expecting the raw URL from the CLI.
In `@crates/ironclaw_reborn_composition/src/ironhub/agent_link.rs`:
- Line 2: The HMAC payload for install delivery is missing signed coverage for
fields that change installation behavior, so a valid signature can be reused
after mutating them. Update the install-delivery signing/verification path in
`agent_link.rs` so `kind` and `private_manifest_url` are included alongside the
other `IronhubInstallDeliveryRequest` fields when building the payload, and make
sure the same canonical field set is used wherever the HMAC is computed and
checked.
In `@crates/ironclaw_reborn_composition/src/ironhub/link_service.rs`:
- Around line 139-154: The HMAC verification in verify_signature/install_payload
is missing install-controlling fields, so the signed payload is not covering
everything later trusted by IronhubLinkService. Update the canonical payload
built by install_payload to include both request.kind and
request.private_manifest_url before verify_signature is called, and add tamper
tests that mutate each field to confirm signature verification fails.
- Around line 86-88: The timestamp_fresh helper currently casts ts to i64 before
validation, which can overflow or wrap on oversized u64 values. Update
timestamp_fresh to reject invalid timestamps by using i64::try_from(ts) first,
then compare the converted value against chrono::Utc::now().timestamp() with
abs_diff (or equivalent safe difference logic) instead of subtraction, so the
freshness check in link_service stays overflow-safe.
In `@crates/ironclaw_reborn_composition/tests/webui_v2_serve.rs`:
- Around line 2017-2043: The register route test only verifies uid and nonce, so
it can miss regressions where aid, ts, or sig are dropped or transformed before
reaching the facade. Update
ironhub_register_route_dispatches_valid_body_to_facade to assert the captured
request in StubServices::ironhub_register_calls includes the full contract from
IronhubRegisterRouteState and ironhub_register_route_mount, checking uid, aid,
ts, nonce, and sig all match the POST body exactly.
In `@crates/ironclaw_webui_v2/tests/webui_v2_handlers_contract.rs`:
- Around line 413-437: The ironhub_deliver_install stub currently returns the
injected error before recording the request, so tests can’t tell whether the
facade was actually called. Update ironhub_deliver_install in the test stub to
push the incoming request into ironhub_deliver_install_calls before checking
next_ironhub_deliver_install_error, while keeping the same injected-error
behavior and preserving the existing request cloning/slug handling.
- Around line 642-668: The happy-path contract test for ironhub install does not
cover private_manifest_url, so a handler could drop that field and still pass.
Update ironhub_deliver_install_dispatches_through_facade to include
private_manifest_url in the POST body and assert it is forwarded into the
StubServices call alongside slug and artifact_digest, using the existing
ironhub_deliver_install_calls capture to verify the facade receives it.
---
Outside diff comments:
In `@crates/ironclaw_reborn_composition/src/ironhub/capabilities.rs`:
- Around line 129-140: Remove private_manifest_url from the model-callable
install input so untrusted requests cannot supply it directly. Update
InstallInput in ironhub/capabilities.rs and any install-path handling around
IronHubInstallOptions so only the trusted signed-link/CLI flow can populate this
field, while model-invoked installs ignore or reject it.
---
Nitpick comments:
In `@crates/ironclaw_product_workflow/src/reborn_services/ironhub_link.rs`:
- Around line 7-12: `IronhubInstallKind` is missing `Serialize`, so the HTTP
wire contract is asymmetric. Update the enum definition in `IronhubInstallKind`
to derive `Serialize` alongside the existing `Deserialize`, keeping the same
`#[serde(rename_all = "snake_case")]` attribute so request/response payloads and
typed clients stay consistent.
In `@crates/ironclaw_reborn_composition/src/ironhub/agent_link.rs`:
- Around line 15-31: Update signature verification to require the validated key
type instead of a raw string: change `verify_signature` (and any related call
sites in the `agent_link` flow) to accept `&IronhubSharedKey`, and use
`IronhubSharedKey::as_str()` internally where the string bytes are needed. This
should make the `IronhubSharedKey::new` length check mandatory for all callers
and prevent bypassing the invariant through plain `&str` inputs.
In `@crates/ironclaw_reborn_composition/src/runtime_input.rs`:
- Around line 255-260: Soften the enablement docstrings in runtime_input.rs for
`ironhub_agent_shared_key` and the related IronHub webhook fields so they
describe intent rather than implying the key alone mounts
`/api/ironhub/register` or enables the full flow. Update the comments near
`ironhub_agent_shared_key`, `ironhub_register_enabled()`, and
`webui_ironhub_link_service()` to mention the additional requirements like
local-runtime lifecycle ports and `host_runtime_http_egress`, keeping the
documentation aligned with the actual guards in code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 028988bd-80b2-42b5-93e1-1e1638d68fe5
📒 Files selected for processing (30)
crates/ironclaw_product_workflow/src/lib.rscrates/ironclaw_product_workflow/src/reborn_services.rscrates/ironclaw_product_workflow/src/reborn_services/ironhub_link.rscrates/ironclaw_reborn_cli/src/commands/ironhub.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_cli/src/runtime/mod.rscrates/ironclaw_reborn_cli/tests/extension.rscrates/ironclaw_reborn_composition/Cargo.tomlcrates/ironclaw_reborn_composition/src/available_extensions.rscrates/ironclaw_reborn_composition/src/ironhub/agent_link.rscrates/ironclaw_reborn_composition/src/ironhub/capabilities.rscrates/ironclaw_reborn_composition/src/ironhub/catalog.rscrates/ironclaw_reborn_composition/src/ironhub/link_service.rscrates/ironclaw_reborn_composition/src/ironhub/mod.rscrates/ironclaw_reborn_composition/src/ironhub/model.rscrates/ironclaw_reborn_composition/src/ironhub/service.rscrates/ironclaw_reborn_composition/src/ironhub/tests.rscrates/ironclaw_reborn_composition/src/ironhub_link_serve.rscrates/ironclaw_reborn_composition/src/lib.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime_input.rscrates/ironclaw_reborn_composition/src/webui.rscrates/ironclaw_reborn_composition/tests/webui_v2_serve.rscrates/ironclaw_webui_v2/CLAUDE.mdcrates/ironclaw_webui_v2/src/descriptors.rscrates/ironclaw_webui_v2/src/handlers.rscrates/ironclaw_webui_v2/src/lib.rscrates/ironclaw_webui_v2/src/router.rscrates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rscrates/ironclaw_webui_v2/tests/webui_v2_handlers_contract.rs
Comment on lines
+1317
to
+1329
| async fn ironhub_deliver_install( | ||
| &self, | ||
| caller: WebUiAuthenticatedCaller, | ||
| request: IronhubInstallDeliveryRequest, | ||
| ) -> Result<IronhubInstallDeliveryResult, RebornServicesError> { | ||
| let service = self | ||
| .ironhub_link | ||
| .as_ref() | ||
| .ok_or_else(ironhub_link::ironhub_link_unavailable)?; | ||
| service | ||
| .deliver_install(caller.user_id, request) | ||
| .await | ||
| .map_err(ironhub_link::map_ironhub_link_error) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Validate the signed install target against the authenticated caller.
This only forwards caller.user_id, so the service never sees the authenticated identity that the signed uid/aid payload is meant to bind. A captured install payload can therefore be consumed by a different authenticated caller before the nonce is spent, which is especially risky for private-manifest installs. Pass the full caller context (or explicit expected UserId/AgentId) into IronhubLinkService::deliver_install and reject mismatches before install. As per coding guidelines, "Fail closed for auth..." and "Preserve tenant/user/agent/project/mission/thread scope..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_product_workflow/src/reborn_services.rs` around lines 1317 -
1329, The `ironhub_deliver_install` flow only passes `caller.user_id` into
`IronhubLinkService::deliver_install`, so the signed install target is not
validated against the authenticated caller context. Update
`ironhub_deliver_install` and the `deliver_install` method on
`IronhubLinkService` to accept the full `WebUiAuthenticatedCaller` or explicit
expected `UserId`/`AgentId`, then compare the signed `uid`/`aid` payload against
the caller before consuming the nonce and reject any mismatch. Keep the auth
check fail-closed and preserve the caller’s tenant/user/agent scope throughout
the install path.
Source: Coding guidelines
| @@ -0,0 +1,167 @@ | |||
| use hmac::{Hmac, Mac}; | |||
| use ironclaw_product_workflow::{IronhubInstallDeliveryRequest, IronhubRegisterRequest}; | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Bind all install-delivery fields into the HMAC payload.
kind and private_manifest_url affect what gets installed and from where, but they are not covered by the signature. A caller with one valid signed delivery can alter those fields without invalidating the HMAC.
Proposed fix
-use ironclaw_product_workflow::{IronhubInstallDeliveryRequest, IronhubRegisterRequest};
+use ironclaw_product_workflow::{
+ IronhubInstallDeliveryRequest, IronhubInstallKind, IronhubRegisterRequest,
+};
pub(super) fn install_payload(request: &IronhubInstallDeliveryRequest) -> String {
+ let kind = request
+ .kind
+ .map(|kind| match kind {
+ IronhubInstallKind::Tool => "tool",
+ IronhubInstallKind::Skill => "skill",
+ })
+ .unwrap_or("");
+ let private_manifest_url = request.private_manifest_url.as_deref().unwrap_or("");
format!(
- "install:{}:{}:{}:{}:{}:{}:{}",
+ "install:{}:{}:{}:{}:{}:{}:{}:{}:{}",
request.slug,
request.version,
request.uid,
request.aid,
request.ts,
request.nonce,
- request.artifact_digest
+ request.artifact_digest,
+ kind,
+ private_manifest_url
)
}Also applies to: 47-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_reborn_composition/src/ironhub/agent_link.rs` at line 2, The
HMAC payload for install delivery is missing signed coverage for fields that
change installation behavior, so a valid signature can be reused after mutating
them. Update the install-delivery signing/verification path in `agent_link.rs`
so `kind` and `private_manifest_url` are included alongside the other
`IronhubInstallDeliveryRequest` fields when building the payload, and make sure
the same canonical field set is used wherever the HMAC is computed and checked.
Comment on lines
+139
to
+154
| if !verify_signature( | ||
| self.shared_key.as_str(), | ||
| &install_payload(&request), | ||
| &request.sig, | ||
| ) { | ||
| return Err(IronhubLinkError::InvalidSignature); | ||
| } | ||
| reject_replayed_nonce(&request.nonce)?; | ||
|
|
||
| let options = IronHubInstallOptions { | ||
| kind: map_kind(request.kind), | ||
| force: false, | ||
| acknowledge_unverified: false, | ||
| expected_version: Some(request.version), | ||
| expected_artifact_digest: Some(request.artifact_digest), | ||
| private_manifest_url: request.private_manifest_url, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sign every field that controls the install.
The verified payload omits kind and private_manifest_url, but both are trusted immediately afterward to choose the package kind/source. Include them in the canonical HMAC payload and add tamper tests for both fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_reborn_composition/src/ironhub/link_service.rs` around lines
139 - 154, The HMAC verification in verify_signature/install_payload is missing
install-controlling fields, so the signed payload is not covering everything
later trusted by IronhubLinkService. Update the canonical payload built by
install_payload to include both request.kind and request.private_manifest_url
before verify_signature is called, and add tamper tests that mutate each field
to confirm signature verification fails.
Cover the private manifest URL in the install HMAC and length-prefix every signed field so the payload stays injective. Keep that URL off the model-callable install path and the CLI argv, drop the unsigned kind field, and reject out-of-range timestamps without overflow. Widen the nonce window past the freshness check and tighten the register and install tests.
Collaborator
|
Thanks @neo-sky — this is good work, and the design here is what we're keeping. Closing this one for mechanical reasons, not because of anything wrong with it. The branch was stacked on Rather than rebase through that, the base (#4479) was re-implemented natively against current main in #6754, and your deep-link/private-manifest work is being re-ported on top of it as the next step. The design is being followed as specified — HMAC-SHA256 register handshake mounted outside bearer auth, install delivery scoped to the authenticated caller's UserId rather than a runtime owner, single-use nonces with drift-checked timestamps, the Private provenance tier failing closed, and manifest replay protection keyed on Your authorship is carried into the replacement commits via Superseded by #6754 (base) and a follow-up PR for this deep-link/private-manifest layer. |
This was referenced Jul 28, 2026
Collaborator
|
The replacement is up: #6780, stacked on #6754. @neo-sky your design is followed as specified — the HMAC-SHA256 register handshake mounted outside bearer auth, caller-scoped install delivery, single-use nonces with drift-checked timestamps, the Private provenance tier failing closed, and manifest replay protection keyed on Two implementation notes you may want to check: HMAC verification uses Review very welcome — you know this trust boundary better than anyone. |
gagdiez
pushed a commit
to gagdiez/ironclaw
that referenced
this pull request
Aug 3, 2026
…nifest source (nearai#6780) * feat(reborn): port IronHub install flow * fix(reborn-ironhub): preserve skill install source and scope on rollback - Preserve URL-sourced skill provenance during forced-replacement rollback. - Restore exact extension installation ownership during compensation. - Reject persisted HostBundled provenance before manifest parsing. - Bound IronHub coordination maps and evict idle keyed locks. - Retain serde error causes in debug logs without changing public error kinds. - Add execute-seam coverage for replacement rollback, integrity checks, and replay rejection. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(reborn-ironhub): skip host-bundled stamps per entry instead of aborting the catalog Fixes an availability regression introduced by e97c124: persisted HostBundled stamps remain rejected, but now skip only the affected extension so valid catalog entries still load. Also removes the IronHub test fixture lint exemption and isolates lock-eviction assertions with fixture-unique identities. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * feat(reborn-ironhub): deep-link register/install gateway + private manifest source Re-port of nearai#5409 onto the current extension-host layout. The original branch was stacked on nearai#4479 and predates the extension-host extraction (nearai#6116, nearai#6616), the removal of ironclaw_product_workflow (nearai#6583), and the webui_v2 crate rename, so the integration is reimplemented against today's APIs rather than rebased. - Public POST /api/ironhub/register handshake (HMAC-SHA256, constant-time via verify_slice) mounted outside bearer auth, plus the bearer-authed ironhub_deliver_install route on the v2 webui surface. - The gateway is disabled by default: it mounts only when IRONHUB_AGENT_SHARED_KEY is set to a non-empty value of at least 16 bytes, and the install-delivery route stays fail-closed as unavailable unless that service is attached. - Install delivery is scoped to the authenticated caller's UserId, so egress runs under the caller rather than a runtime owner. - Install nonces are single-use and consumed durably (keyed by SHA-256 digest, bounded length, control characters rejected); timestamps are drift-checked within a 300-second window. - Private manifest source: Ed25519-signed org-scoped manifests install from the configured catalog host behind a Private provenance tier that is rejected unless the install genuinely came from a private manifest. - Manifest replay/downgrade protection keys on (host, signed repo) so it is independent of the rotating per-install access token in the URL. Link logic lives in ironclaw_extension_host::ironhub (agent_link, link_service); the product-layer service moved to ironclaw_product::reborn_services::ironhub_link now that product_workflow is gone; serve wiring is composition-owned. Supersedes nearai#5409. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(turns): trust verified catalog descriptions instead of denying the whole prompt Installing Attio from the signed IronHub catalog exposed an official description containing API key and Bearer authentication vocabulary. Prompt validation treated that trusted text as an unsafe summary and denied every subsequent turn. Carry verified catalog provenance into capability descriptors and route it through a trusted prompt-text surface, following the certified-skill fix from nearai#5169/nearai#5258. Structural checks still apply, while invalid untrusted descriptors are omitted individually with host diagnostics naming the capability and matched pattern. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * test(golden): re-bless capability surface hashes after the description-trust field Adding CapabilityDescriptionTrust to CapabilityDescriptorView changes the capability surface fingerprint, so the golden payload snapshots carry a new surface sha256. Verified the change is hash-only: all 7 changed content lines are byte-identical once the surface hash is normalized, with zero other content deltas. The trust field does not appear in model-visible prompt content — the capability list and every description are unchanged. Only insta's stale assertion_line metadata was additionally dropped. * fix(ironhub): return complete, self-describing search results instead of a silent truncation (nearai#6808) IronHub search returned a result-reference prefix, leading the agent to report attio missing even though it was present in the signed catalog. Return compact catalog projections with explicit completeness metadata and a bounded, unmistakably incomplete fallback. Closes nearai#6788 Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * test(integration): cover the install-then-turn prompt-denial incident at the turn seam Drive registry-verified and local extension descriptions through the real product workflow, scheduler, agent loop, and model request boundary. This closes the Attio incident gap by proving verified Bearer-header wording survives intact while one unsafe local prompt entry degrades without collapsing the remaining capability surface. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): measure catalog_total inside the truncation budget Follow-up to b218c05. The bounded-fallback path sized `Self::incomplete(...)` against MAX_SEARCH_RESPONSE_BYTES while that shape still had `catalog_total: None` — omitted from JSON by skip_serializing_if — and then assigned Some(..) to the value actually returned. The emitted payload was therefore ~20 bytes larger than the budget that admitted it, so a truncated result could exceed the cap it exists to enforce. `incomplete` now takes `catalog_total`, so the shape measured is exactly the shape emitted. Extended the existing oversized-catalog test rather than adding a fourth search test (it already owns the truncated path): it now pins catalog_total on both the struct and the serialized payload, alongside its existing size bound. * fix(host_api): redact credentials in model previews instead of dropping them A deployed agent could not return the IronHub catalog. The payload was fine — 13,755 bytes, complete, well under every size bound. It never reached the model. `result_preview_parts` built the preview, then discarded it because `ModelResultPreview::new` refuses any content containing a credential marker ("access token", "api key", "bearer ", "password", "secret", ...). One catalog entry's summary says "no API key" — describing the ABSENCE of one — and that phrase refused the entire catalog. The caller's `else` arm drops the preview AND the continuation metadata that travels with it, so the model received a bare result reference with no preview, no total_bytes and no next_offset: unreadable and unpageable. The logs show it then trying result_read (which returned a reference to a reference), curl, wget, python3, an HTTP fetch that 404'd, and five more identical searches. Masking, not refusal, for model-visible CONTENT: - `credential_redaction::redact_credential_text` masks credential markers (at a word boundary, so "Secretary" survives) and credential-shaped tokens (sk-, ghp_, AKIA...) with [redacted], preserving everything else. - `ModelResultPreview::redacted` falls back to the masked text when the strict contract refuses; `ModelResultPreview::new` is unchanged for callers that can legitimately reject an operation. - The preview path uses it, so content and its continuation metadata survive. The security property is unchanged: credential material still never reaches the model. Only the disposal changed — mask the span rather than discard the payload. The existing resolution test now asserts exactly that: the secret is absent from the preview AND the surrounding content survives. REVISIT: the marker list is credential *vocabulary*, so prose like "no API key required" is masked despite containing no credential. `contains_unredacted_credential_value` already models the sharper "label followed by a value" rule and its own doc notes that vocabulary alone is valid diagnostic context. Narrowing this is a separate decision about a shared credential boundary and is deliberately not made here — masking is strictly better than today's wholesale refusal. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(host_api): share the marker boundary rule so redaction masks every marker 4b85acc added credential masking but reimplemented the marker boundary check instead of reusing the detector's, and got it wrong: it required an alphanumeric boundary on BOTH sides for every marker. Markers that already carry a delimiter ("bearer ", "authorization:") therefore never matched — "presented as a Bearer header" left `bearer ` untouched, `contains_credential_marker` still returned true, validation still failed, and the preview was still dropped. Net effect: the fix did not fix the production case. An `extension_search` result for attio (1088 bytes) still reached the model as a bare reference with no preview, and a `result_read` on it returned another reference whose own read failed with "result reference is unavailable in this thread". `marker_match_at` is now extracted from `contains_marker_at_word_boundary` and used by BOTH the detector and the redactor, so the two cannot drift again: a marker carrying its own delimiter skips the boundary check on that side. The seam regression now uses the real production string ("Authenticated with a workspace API key presented as a Bearer header") rather than a single-marker stand-in. That string is what the earlier test missed: it contained only "api key", which masked correctly, so the bug hid behind a passing test. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): carry published credential recipes into the generated manifest An IronHub tool that needs a credential installed "successfully" and could never authenticate. Attio is the reported case: it was installed, activated, and callable as attio.invoke, but nothing in the extension model knew an API key was required, so no auth challenge was raised and the in-chat credential card never rendered. The agent filled the gap by inventing a CLI command. Cause: `generic_tool_manifest` synthesised the v3 manifest from the catalog entry's name/version/description alone and hardcoded `effects = ["network"]`. The tool's own credential recipe travels in the capabilities artifact — already downloaded, digest-verified, and written into the package as `legacy/capabilities.json` — and was never read. Attio publishes: "http": { "credentials": { "attio_api_key": { "location": { "type": "bearer" }, "host_patterns": ["api.attio.com"] } } } `mapped_credentials` now reads that and emits a `[[tools.credentials]]` block plus the `use_secret` effect, matching how bundled first-party extensions (github, slack) declare credentials. Location mapping, from a survey of all nine credentialed catalog tools: - bearer (7 tools) -> header "authorization" with prefix "Bearer " - header (monday) -> that header name, NO prefix; monday.com sends the raw token as the Authorization value, so an invented prefix would break every request - basic (wazuh) -> UNSUPPORTED: v3 injection models header/query/path/pointer, not HTTP Basic. Fails the install with a message naming the tool and the location type, rather than repeating the silent-success failure this change fixes. Policy fields stay host-authored. `trust`, `origin_gate_matrix`, `default_permission` and `visibility` remain hardcoded: a third-party package declares which credential it needs, never what it is allowed to do. That is why generation is kept and the tool's own manifest.toml is still not trusted. Tests cover each location shape, the credential-free path (unchanged output), and that the generated manifest parses as real v3 with the credential block and use_secret effect present — not just that the string contains them. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): emit the [auth.<vendor>] recipe credentials require e53ecc0 propagated credential recipes into the generated manifest but omitted the vendor auth recipe, so every credentialed IronHub install failed: credential vendor `attio` has no [auth.attio] recipe; v3 manifests must declare one for every referenced vendor That surfaced to the user as `ironhub_install` -> operation_failed with no diagnostic detail, which is worse than the bug it replaced: before, install "succeeded" and could not authenticate; after, install failed opaquely. The generated manifest now carries `[auth.<vendor>]` with `method = "api_key"` — the variant that maps to RuntimeCredentialAccountSetup::ManualToken, which is the flow that renders the masked in-chat credential card. display_name and the per-field labels come from the tool's own published `auth` and `setup.required_secrets` blocks, so the user sees the vendor's own wording. `validation` is omitted deliberately: it is optional, and a probe the host invents could fail against a service it has never contacted. Why this shipped: the previous test asserted the manifest was valid TOML and contained the right keys. It was, and it did — but `registry_extension_package` runs the production v3 parser and package validation, which the string test never reached. The new test drives `ironhub_tool_package` (the real caller seam) with a real WASI component fixture, so manifest validation is actually exercised. Per .claude/rules/testing.md: test through the caller, not the helper. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): derive the auth method from the tool, not a hardcoded api_key dfff895 emitted `method = "api_key"` for every credentialed tool. Four catalog tools (gitlab, clickup, microsoft-365, xero) are genuinely OAuth2 and publish an `auth.oauth` block; forcing api_key on them means the user pastes an access token by hand that then expires with no refresh — gitlab's own description promises "host-managed token refresh". The method now follows what the tool published: - `auth.oauth` present -> `method = "oauth2_code"`, carrying authorization/token endpoints, the scope ceiling, PKCE (S256 default, explicit "none" only on opt-out), and client_id_env/client_secret_env as deployment secret HANDLES so no secret material enters the manifest. - absent -> `method = "api_key"`, which maps to RuntimeCredentialAccountSetup::ManualToken and renders the masked in-chat card. `token_response` is required by the recipe but absent from the capabilities artifact, so it is synthesised as /access_token, /refresh_token, /expires_in. Unlike the `validation` probe (a URL only the vendor can know, still omitted), this shape is defined by RFC 6749 section 5.1 and implemented by every OAuth2 vendor in the catalog; the pointers declare where to look, not that the fields must be present. `identity`, `refresh` and `revoke` stay absent — all optional. Both arms are now proven through `ironhub_tool_package`, the production package validator, with a real WASI component fixture. The api_key arm alone would have kept the four OAuth tools broken in a way string assertions could not catch: the oauth2_code recipe has stricter requirements than api_key, and it was exactly `missing field token_response` that the caller-seam test surfaced. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * docs(ironhub): name the supported credential locations in the failure The fail-closed arm now lists what the host can inject ('bearer', 'header') and records why the rest are absent: QueryParam/PathPlaceholder/BodyJsonPointer are modelled by RuntimeCredentialTarget but published by no catalog tool, so their tool-side spelling is unverified and mapping them now would be speculative; 'basic' is genuinely inexpressible in v3 injection, which has no HTTP Basic variant. * refactor(extension-host): key persisted manifest sources by ExtensionId manifest_sources was BTreeMap<String, ManifestSource>, introduced by aabbc70. The construction site in factory.rs already held a validated ExtensionId and threw the type away (`record.manifest().id.as_str().to_string()`), so an unnormalized key would silently miss every lookup rather than fail — the exact class .claude/rules/types.md exists to prevent. Keyed by ExtensionId end to end: the boundary keeps the typed identity, the catalog lookup drops `.as_str()`, and the two test fixtures construct real ids. Contained to 2 files; no behavior change, and the per-entry host-bundled provenance regression still passes. * feat(ironhub): install from the published manifest and carry its setup steps An IronHub tool did not ship the manifest IronClaw installs from. IronClaw built one at install time by string-concatenating TOML from `capabilities.json`, a schema this repository does not own. That reconstruction lost fields silently three times — the credential blocks, then the `[auth.<vendor>]` recipe those credentials referenced, then the OAuth versus API-key distinction — and each loss reached a user as an install that could not authenticate, because the only machine the translation ran on was theirs. nearai/ironhub#254 publishes the manifest as a signed catalog artifact. Install from it: - `IronHubToolEntry` gains an optional `manifest` artifact, downloaded and digest-verified like the wasm and capabilities. Optional so a catalog predating published manifests still lists every tool; installing one of those is what fails, naming the cause. - `ironhub_tool_package` places the wasm and the two host-owned generic schemas at the paths the manifest declares, so publisher and host never have to agree a filename convention across two repositories. `crate_name` existed only to build those paths and is gone. - The 349-line translator is deleted, along with the catalog field it needed. - A manifest whose id contradicts the catalog entry is refused rather than resolved in the manifest's favour: installing `github` when the user asked for `attio` would shadow an unrelated extension. The second half is what a user actually feels. `capabilities.json` already recorded how to obtain each credential — Attio's says to open Workspace Settings > Developers and create an access token, with the URL beside it — but the auth recipes had nowhere to put it, so an installed tool could say it needed a secret and nothing about where the secret comes from. Users had no way to activate what they had just installed, and models asked for help invented the steps. `ApiKeyRecipe` and `OAuth2CodeRecipe` gain optional `instructions` and `setup_url`, and the shared import seam turns them into the onboarding copy the extensions UI already knows how to render. Deriving it at that seam means uploaded packages get it too, not just registry ones. `setup_url` is an `HttpsEndpoint` because it becomes a link the user is invited to follow; the text is rendered through JSX interpolation and never reaches model-visible prompt text. Verified against the live IronHub catalog: all 17 publishable tools install through the production package validator, 8 of them now carrying their vendor's real setup steps (the other 9 declare no `auth.instructions` upstream). Attio surfaces "open Workspace Settings > Developers ..." and its settings URL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ironhub): stop naming a concrete extension in generic package code The extension-specificity gate flags a first-party extension name appearing in generic code. The identity-pin comment used two real extensions to illustrate the shadowing it prevents, and the regression test built its contradicting manifest under a real extension id. Neither needed to be a real name: the rule is about an id the user did not ask for, whichever id that is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(architecture): extract the IronHub client into ironclaw_ironhub (nearai#6870) crates/AGENTS.md gives ironclaw_extension_host an explicit non-goal: "Host authority (signing secrets, bot tokens, network egress)". The ironhub module inside it did network egress (catalog and artifact downloads), held the pinned Ed25519 catalog trust anchor, and owned the HMAC deep-link shared key — plus skill installs, which are not extension lifecycle at all. It landed there because the seam it drives (registry_extension_package) lives there, not because the crate owns the concern; the boundary tests never saw it because no dependency edge changed. Move the module wholesale to a new crate directly above extension_host. The placement rule it restores: generic registry seams (registry_extension_package, parse_imported_manifest, ManifestSource::RegistryInstalled) stay in extension_host, and the one concrete catalog client is vendor-scoped by charter, the same way each concrete extension crate is scoped to its product. The module already touched its host crate through exactly four public symbols, so extension_host's API is unchanged apart from deleting `pub mod ironhub` and dropping ed25519-dalek, which nothing else in the crate used. Wiring changes, all shape-preserving: - The binary's exact dependency allowlist stays closed: composition re-exports the command vocabulary as `ironclaw_reborn_composition::ironhub` with the house consumer-and-test doc comment, and the CLI imports the facade instead of reaching extension_host. - ironclaw_ironhub gets its dependency BoundaryRule in the same PR (a new crate is unruled by default): no execution runtimes, no secret storage, no serve/assembly layers, no concrete extension crates. - extension_host's `test-support` feature now forwards the filesystem/host_api/processes test-support seams and exposes lifecycle_test_support behind it, so the moved integration-style tests drive the same real lifecycle services from outside the crate. - Fixture include paths shorten by one directory level; the moved files are otherwise verbatim (git tracks them as renames). Verified: full architecture suite (18/18 result sets), ironclaw_ironhub 34/34, workspace clippy clean with -D warnings. Composition lib tests: 638 passed, with two known parallel-execution flakes that pass serially and one pre-existing trace-capture failure unrelated to this move (fails identically with the module in either location). Co-authored-by: serrrfirat <firatsertgoz@alumni.sabanciuniv.edu> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(ironhub): accept hub-prefixed artifact digests * fix(ironhub): address review feedback * fix(ironhub): propagate manifest URL to CLI commands * fix(ironhub): close rereview findings * fix(composition): preserve configured IronHub catalog * fix(ironhub): restore failed skill replacements safely * fix(ironhub): restore complete skill bundles * fix(skills): preserve restore failure context * test(ironhub): cover mediated service entrypoints * fix(ironhub): install verified tool schemas * test(coverage): recapture extension host after IronHub extraction * test(coverage): track IronHub changed-code gaps * test(ironhub): execute reviewed coverage paths * test(ironhub): close changed coverage gap * test(composition): pin IronHub register default-off * Fix changed coverage exemption lines * fix(ironhub): address replay persistence review * test(ironhub): cover install error paths * ci: rebase changed coverage exemptions * fix(ironhub): share durable link state across surfaces * ci(coverage): rebase IronHub runtime exemptions * fix: refresh capabilities after IronHub install * test: remove stale extension host lifecycle fixture * fix: use canonical extension schemas in the loop * fix(ironhub): address coderabbit review — harden shared key validation (nearai#6780) * fix(ci): update IronHub fixture paths after package move * fix(ci): preserve selected integration coverage mode * fix(ci): keep MSRV override for selected integration lanes * fix(extensions): address think-in-universe review — provider schemas and setup copy (nearai#6780) --------- Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> Co-authored-by: serrrfirat <firatsertgoz@alumni.sabanciuniv.edu> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
18 of 21 tasks
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
…nifest source (nearai#6780) * feat(reborn): port IronHub install flow * fix(reborn-ironhub): preserve skill install source and scope on rollback - Preserve URL-sourced skill provenance during forced-replacement rollback. - Restore exact extension installation ownership during compensation. - Reject persisted HostBundled provenance before manifest parsing. - Bound IronHub coordination maps and evict idle keyed locks. - Retain serde error causes in debug logs without changing public error kinds. - Add execute-seam coverage for replacement rollback, integrity checks, and replay rejection. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(reborn-ironhub): skip host-bundled stamps per entry instead of aborting the catalog Fixes an availability regression introduced by e97c124: persisted HostBundled stamps remain rejected, but now skip only the affected extension so valid catalog entries still load. Also removes the IronHub test fixture lint exemption and isolates lock-eviction assertions with fixture-unique identities. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * feat(reborn-ironhub): deep-link register/install gateway + private manifest source Re-port of nearai#5409 onto the current extension-host layout. The original branch was stacked on nearai#4479 and predates the extension-host extraction (nearai#6116, nearai#6616), the removal of ironclaw_product_workflow (nearai#6583), and the webui_v2 crate rename, so the integration is reimplemented against today's APIs rather than rebased. - Public POST /api/ironhub/register handshake (HMAC-SHA256, constant-time via verify_slice) mounted outside bearer auth, plus the bearer-authed ironhub_deliver_install route on the v2 webui surface. - The gateway is disabled by default: it mounts only when IRONHUB_AGENT_SHARED_KEY is set to a non-empty value of at least 16 bytes, and the install-delivery route stays fail-closed as unavailable unless that service is attached. - Install delivery is scoped to the authenticated caller's UserId, so egress runs under the caller rather than a runtime owner. - Install nonces are single-use and consumed durably (keyed by SHA-256 digest, bounded length, control characters rejected); timestamps are drift-checked within a 300-second window. - Private manifest source: Ed25519-signed org-scoped manifests install from the configured catalog host behind a Private provenance tier that is rejected unless the install genuinely came from a private manifest. - Manifest replay/downgrade protection keys on (host, signed repo) so it is independent of the rotating per-install access token in the URL. Link logic lives in ironclaw_extension_host::ironhub (agent_link, link_service); the product-layer service moved to ironclaw_product::reborn_services::ironhub_link now that product_workflow is gone; serve wiring is composition-owned. Supersedes nearai#5409. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(turns): trust verified catalog descriptions instead of denying the whole prompt Installing Attio from the signed IronHub catalog exposed an official description containing API key and Bearer authentication vocabulary. Prompt validation treated that trusted text as an unsafe summary and denied every subsequent turn. Carry verified catalog provenance into capability descriptors and route it through a trusted prompt-text surface, following the certified-skill fix from nearai#5169/nearai#5258. Structural checks still apply, while invalid untrusted descriptors are omitted individually with host diagnostics naming the capability and matched pattern. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * test(golden): re-bless capability surface hashes after the description-trust field Adding CapabilityDescriptionTrust to CapabilityDescriptorView changes the capability surface fingerprint, so the golden payload snapshots carry a new surface sha256. Verified the change is hash-only: all 7 changed content lines are byte-identical once the surface hash is normalized, with zero other content deltas. The trust field does not appear in model-visible prompt content — the capability list and every description are unchanged. Only insta's stale assertion_line metadata was additionally dropped. * fix(ironhub): return complete, self-describing search results instead of a silent truncation (nearai#6808) IronHub search returned a result-reference prefix, leading the agent to report attio missing even though it was present in the signed catalog. Return compact catalog projections with explicit completeness metadata and a bounded, unmistakably incomplete fallback. Closes nearai#6788 Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * test(integration): cover the install-then-turn prompt-denial incident at the turn seam Drive registry-verified and local extension descriptions through the real product workflow, scheduler, agent loop, and model request boundary. This closes the Attio incident gap by proving verified Bearer-header wording survives intact while one unsafe local prompt entry degrades without collapsing the remaining capability surface. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): measure catalog_total inside the truncation budget Follow-up to b218c05. The bounded-fallback path sized `Self::incomplete(...)` against MAX_SEARCH_RESPONSE_BYTES while that shape still had `catalog_total: None` — omitted from JSON by skip_serializing_if — and then assigned Some(..) to the value actually returned. The emitted payload was therefore ~20 bytes larger than the budget that admitted it, so a truncated result could exceed the cap it exists to enforce. `incomplete` now takes `catalog_total`, so the shape measured is exactly the shape emitted. Extended the existing oversized-catalog test rather than adding a fourth search test (it already owns the truncated path): it now pins catalog_total on both the struct and the serialized payload, alongside its existing size bound. * fix(host_api): redact credentials in model previews instead of dropping them A deployed agent could not return the IronHub catalog. The payload was fine — 13,755 bytes, complete, well under every size bound. It never reached the model. `result_preview_parts` built the preview, then discarded it because `ModelResultPreview::new` refuses any content containing a credential marker ("access token", "api key", "bearer ", "password", "secret", ...). One catalog entry's summary says "no API key" — describing the ABSENCE of one — and that phrase refused the entire catalog. The caller's `else` arm drops the preview AND the continuation metadata that travels with it, so the model received a bare result reference with no preview, no total_bytes and no next_offset: unreadable and unpageable. The logs show it then trying result_read (which returned a reference to a reference), curl, wget, python3, an HTTP fetch that 404'd, and five more identical searches. Masking, not refusal, for model-visible CONTENT: - `credential_redaction::redact_credential_text` masks credential markers (at a word boundary, so "Secretary" survives) and credential-shaped tokens (sk-, ghp_, AKIA...) with [redacted], preserving everything else. - `ModelResultPreview::redacted` falls back to the masked text when the strict contract refuses; `ModelResultPreview::new` is unchanged for callers that can legitimately reject an operation. - The preview path uses it, so content and its continuation metadata survive. The security property is unchanged: credential material still never reaches the model. Only the disposal changed — mask the span rather than discard the payload. The existing resolution test now asserts exactly that: the secret is absent from the preview AND the surrounding content survives. REVISIT: the marker list is credential *vocabulary*, so prose like "no API key required" is masked despite containing no credential. `contains_unredacted_credential_value` already models the sharper "label followed by a value" rule and its own doc notes that vocabulary alone is valid diagnostic context. Narrowing this is a separate decision about a shared credential boundary and is deliberately not made here — masking is strictly better than today's wholesale refusal. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(host_api): share the marker boundary rule so redaction masks every marker 4b85acc added credential masking but reimplemented the marker boundary check instead of reusing the detector's, and got it wrong: it required an alphanumeric boundary on BOTH sides for every marker. Markers that already carry a delimiter ("bearer ", "authorization:") therefore never matched — "presented as a Bearer header" left `bearer ` untouched, `contains_credential_marker` still returned true, validation still failed, and the preview was still dropped. Net effect: the fix did not fix the production case. An `extension_search` result for attio (1088 bytes) still reached the model as a bare reference with no preview, and a `result_read` on it returned another reference whose own read failed with "result reference is unavailable in this thread". `marker_match_at` is now extracted from `contains_marker_at_word_boundary` and used by BOTH the detector and the redactor, so the two cannot drift again: a marker carrying its own delimiter skips the boundary check on that side. The seam regression now uses the real production string ("Authenticated with a workspace API key presented as a Bearer header") rather than a single-marker stand-in. That string is what the earlier test missed: it contained only "api key", which masked correctly, so the bug hid behind a passing test. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): carry published credential recipes into the generated manifest An IronHub tool that needs a credential installed "successfully" and could never authenticate. Attio is the reported case: it was installed, activated, and callable as attio.invoke, but nothing in the extension model knew an API key was required, so no auth challenge was raised and the in-chat credential card never rendered. The agent filled the gap by inventing a CLI command. Cause: `generic_tool_manifest` synthesised the v3 manifest from the catalog entry's name/version/description alone and hardcoded `effects = ["network"]`. The tool's own credential recipe travels in the capabilities artifact — already downloaded, digest-verified, and written into the package as `legacy/capabilities.json` — and was never read. Attio publishes: "http": { "credentials": { "attio_api_key": { "location": { "type": "bearer" }, "host_patterns": ["api.attio.com"] } } } `mapped_credentials` now reads that and emits a `[[tools.credentials]]` block plus the `use_secret` effect, matching how bundled first-party extensions (github, slack) declare credentials. Location mapping, from a survey of all nine credentialed catalog tools: - bearer (7 tools) -> header "authorization" with prefix "Bearer " - header (monday) -> that header name, NO prefix; monday.com sends the raw token as the Authorization value, so an invented prefix would break every request - basic (wazuh) -> UNSUPPORTED: v3 injection models header/query/path/pointer, not HTTP Basic. Fails the install with a message naming the tool and the location type, rather than repeating the silent-success failure this change fixes. Policy fields stay host-authored. `trust`, `origin_gate_matrix`, `default_permission` and `visibility` remain hardcoded: a third-party package declares which credential it needs, never what it is allowed to do. That is why generation is kept and the tool's own manifest.toml is still not trusted. Tests cover each location shape, the credential-free path (unchanged output), and that the generated manifest parses as real v3 with the credential block and use_secret effect present — not just that the string contains them. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): emit the [auth.<vendor>] recipe credentials require e53ecc0 propagated credential recipes into the generated manifest but omitted the vendor auth recipe, so every credentialed IronHub install failed: credential vendor `attio` has no [auth.attio] recipe; v3 manifests must declare one for every referenced vendor That surfaced to the user as `ironhub_install` -> operation_failed with no diagnostic detail, which is worse than the bug it replaced: before, install "succeeded" and could not authenticate; after, install failed opaquely. The generated manifest now carries `[auth.<vendor>]` with `method = "api_key"` — the variant that maps to RuntimeCredentialAccountSetup::ManualToken, which is the flow that renders the masked in-chat credential card. display_name and the per-field labels come from the tool's own published `auth` and `setup.required_secrets` blocks, so the user sees the vendor's own wording. `validation` is omitted deliberately: it is optional, and a probe the host invents could fail against a service it has never contacted. Why this shipped: the previous test asserted the manifest was valid TOML and contained the right keys. It was, and it did — but `registry_extension_package` runs the production v3 parser and package validation, which the string test never reached. The new test drives `ironhub_tool_package` (the real caller seam) with a real WASI component fixture, so manifest validation is actually exercised. Per .claude/rules/testing.md: test through the caller, not the helper. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * fix(ironhub): derive the auth method from the tool, not a hardcoded api_key dfff895 emitted `method = "api_key"` for every credentialed tool. Four catalog tools (gitlab, clickup, microsoft-365, xero) are genuinely OAuth2 and publish an `auth.oauth` block; forcing api_key on them means the user pastes an access token by hand that then expires with no refresh — gitlab's own description promises "host-managed token refresh". The method now follows what the tool published: - `auth.oauth` present -> `method = "oauth2_code"`, carrying authorization/token endpoints, the scope ceiling, PKCE (S256 default, explicit "none" only on opt-out), and client_id_env/client_secret_env as deployment secret HANDLES so no secret material enters the manifest. - absent -> `method = "api_key"`, which maps to RuntimeCredentialAccountSetup::ManualToken and renders the masked in-chat card. `token_response` is required by the recipe but absent from the capabilities artifact, so it is synthesised as /access_token, /refresh_token, /expires_in. Unlike the `validation` probe (a URL only the vendor can know, still omitted), this shape is defined by RFC 6749 section 5.1 and implemented by every OAuth2 vendor in the catalog; the pointers declare where to look, not that the fields must be present. `identity`, `refresh` and `revoke` stay absent — all optional. Both arms are now proven through `ironhub_tool_package`, the production package validator, with a real WASI component fixture. The api_key arm alone would have kept the four OAuth tools broken in a way string assertions could not catch: the oauth2_code recipe has stricter requirements than api_key, and it was exactly `missing field token_response` that the caller-seam test surfaced. Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> * docs(ironhub): name the supported credential locations in the failure The fail-closed arm now lists what the host can inject ('bearer', 'header') and records why the rest are absent: QueryParam/PathPlaceholder/BodyJsonPointer are modelled by RuntimeCredentialTarget but published by no catalog tool, so their tool-side spelling is unverified and mapping them now would be speculative; 'basic' is genuinely inexpressible in v3 injection, which has no HTTP Basic variant. * refactor(extension-host): key persisted manifest sources by ExtensionId manifest_sources was BTreeMap<String, ManifestSource>, introduced by aabbc70. The construction site in factory.rs already held a validated ExtensionId and threw the type away (`record.manifest().id.as_str().to_string()`), so an unnormalized key would silently miss every lookup rather than fail — the exact class .claude/rules/types.md exists to prevent. Keyed by ExtensionId end to end: the boundary keeps the typed identity, the catalog lookup drops `.as_str()`, and the two test fixtures construct real ids. Contained to 2 files; no behavior change, and the per-entry host-bundled provenance regression still passes. * feat(ironhub): install from the published manifest and carry its setup steps An IronHub tool did not ship the manifest IronClaw installs from. IronClaw built one at install time by string-concatenating TOML from `capabilities.json`, a schema this repository does not own. That reconstruction lost fields silently three times — the credential blocks, then the `[auth.<vendor>]` recipe those credentials referenced, then the OAuth versus API-key distinction — and each loss reached a user as an install that could not authenticate, because the only machine the translation ran on was theirs. nearai/ironhub#254 publishes the manifest as a signed catalog artifact. Install from it: - `IronHubToolEntry` gains an optional `manifest` artifact, downloaded and digest-verified like the wasm and capabilities. Optional so a catalog predating published manifests still lists every tool; installing one of those is what fails, naming the cause. - `ironhub_tool_package` places the wasm and the two host-owned generic schemas at the paths the manifest declares, so publisher and host never have to agree a filename convention across two repositories. `crate_name` existed only to build those paths and is gone. - The 349-line translator is deleted, along with the catalog field it needed. - A manifest whose id contradicts the catalog entry is refused rather than resolved in the manifest's favour: installing `github` when the user asked for `attio` would shadow an unrelated extension. The second half is what a user actually feels. `capabilities.json` already recorded how to obtain each credential — Attio's says to open Workspace Settings > Developers and create an access token, with the URL beside it — but the auth recipes had nowhere to put it, so an installed tool could say it needed a secret and nothing about where the secret comes from. Users had no way to activate what they had just installed, and models asked for help invented the steps. `ApiKeyRecipe` and `OAuth2CodeRecipe` gain optional `instructions` and `setup_url`, and the shared import seam turns them into the onboarding copy the extensions UI already knows how to render. Deriving it at that seam means uploaded packages get it too, not just registry ones. `setup_url` is an `HttpsEndpoint` because it becomes a link the user is invited to follow; the text is rendered through JSX interpolation and never reaches model-visible prompt text. Verified against the live IronHub catalog: all 17 publishable tools install through the production package validator, 8 of them now carrying their vendor's real setup steps (the other 9 declare no `auth.instructions` upstream). Attio surfaces "open Workspace Settings > Developers ..." and its settings URL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ironhub): stop naming a concrete extension in generic package code The extension-specificity gate flags a first-party extension name appearing in generic code. The identity-pin comment used two real extensions to illustrate the shadowing it prevents, and the regression test built its contradicting manifest under a real extension id. Neither needed to be a real name: the rule is about an id the user did not ask for, whichever id that is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(architecture): extract the IronHub client into ironclaw_ironhub (nearai#6870) crates/AGENTS.md gives ironclaw_extension_host an explicit non-goal: "Host authority (signing secrets, bot tokens, network egress)". The ironhub module inside it did network egress (catalog and artifact downloads), held the pinned Ed25519 catalog trust anchor, and owned the HMAC deep-link shared key — plus skill installs, which are not extension lifecycle at all. It landed there because the seam it drives (registry_extension_package) lives there, not because the crate owns the concern; the boundary tests never saw it because no dependency edge changed. Move the module wholesale to a new crate directly above extension_host. The placement rule it restores: generic registry seams (registry_extension_package, parse_imported_manifest, ManifestSource::RegistryInstalled) stay in extension_host, and the one concrete catalog client is vendor-scoped by charter, the same way each concrete extension crate is scoped to its product. The module already touched its host crate through exactly four public symbols, so extension_host's API is unchanged apart from deleting `pub mod ironhub` and dropping ed25519-dalek, which nothing else in the crate used. Wiring changes, all shape-preserving: - The binary's exact dependency allowlist stays closed: composition re-exports the command vocabulary as `ironclaw_reborn_composition::ironhub` with the house consumer-and-test doc comment, and the CLI imports the facade instead of reaching extension_host. - ironclaw_ironhub gets its dependency BoundaryRule in the same PR (a new crate is unruled by default): no execution runtimes, no secret storage, no serve/assembly layers, no concrete extension crates. - extension_host's `test-support` feature now forwards the filesystem/host_api/processes test-support seams and exposes lifecycle_test_support behind it, so the moved integration-style tests drive the same real lifecycle services from outside the crate. - Fixture include paths shorten by one directory level; the moved files are otherwise verbatim (git tracks them as renames). Verified: full architecture suite (18/18 result sets), ironclaw_ironhub 34/34, workspace clippy clean with -D warnings. Composition lib tests: 638 passed, with two known parallel-execution flakes that pass serially and one pre-existing trace-capture failure unrelated to this move (fails identically with the module in either location). Co-authored-by: serrrfirat <firatsertgoz@alumni.sabanciuniv.edu> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(ironhub): accept hub-prefixed artifact digests * fix(ironhub): address review feedback * fix(ironhub): propagate manifest URL to CLI commands * fix(ironhub): close rereview findings * fix(composition): preserve configured IronHub catalog * fix(ironhub): restore failed skill replacements safely * fix(ironhub): restore complete skill bundles * fix(skills): preserve restore failure context * test(ironhub): cover mediated service entrypoints * fix(ironhub): install verified tool schemas * test(coverage): recapture extension host after IronHub extraction * test(coverage): track IronHub changed-code gaps * test(ironhub): execute reviewed coverage paths * test(ironhub): close changed coverage gap * test(composition): pin IronHub register default-off * Fix changed coverage exemption lines * fix(ironhub): address replay persistence review * test(ironhub): cover install error paths * ci: rebase changed coverage exemptions * fix(ironhub): share durable link state across surfaces * ci(coverage): rebase IronHub runtime exemptions * fix: refresh capabilities after IronHub install * test: remove stale extension host lifecycle fixture * fix: use canonical extension schemas in the loop * fix(ironhub): address coderabbit review — harden shared key validation (nearai#6780) * fix(ci): update IronHub fixture paths after package move * fix(ci): preserve selected integration coverage mode * fix(ci): keep MSRV override for selected integration lanes * fix(extensions): address think-in-universe review — provider schemas and setup copy (nearai#6780) --------- Co-authored-by: neo-sky <brandon.m.henderson93@gmail.com> Co-authored-by: serrrfirat <firatsertgoz@alumni.sabanciuniv.edu> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This builds on
codex/ironhub-reborn-port. It ports the IronHub deep-link register/install gateway onto the Reborn surface and adds the private org-scoped manifest install path, so the hub can register an agent over HMAC and deliver private install intents.What's here:
POST /api/ironhub/registerwebhook (HMAC-SHA256 handshake) mounted outside bearer auth, plus thewebui.v2.ironhub_deliver_installroute on the bearer-authed v2 surface.(host, signed repo), so it is independent of the rotating per-install access token in the URL.Latest commit is review hardening: a single-source register-enabled gate shared by serve and the facade attach, shared-key length validation, verifier and host-pinning annotations, and tests covering link-service negative paths (stale/bad-sig/replay/kind-map), the register handler, provenance rejection, manifest replay rejection, and the webui_v2 handler.
Gate: fmt + clippy
-D warnings+ tests green on the touched crates (composition, webui_v2, cli, product_workflow).