Skip to content

fix(safety): redact model-bound secrets without rejecting turns - #7509

Merged
serrrfirat merged 22 commits into
mainfrom
codex/fix-thread-recovery-prompt-denylist
Aug 12, 2026
Merged

serrrfirat merged 22 commits into
mainfrom
codex/fix-thread-recovery-prompt-denylist

Conversation

@serrrfirat

@serrrfirat serrrfirat commented Aug 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Replace credential-content rejection in prompt contracts with deterministic redaction, so one false positive can no longer block prompt construction, attachment recovery, or unrelated threads.
  • Add a source-independent final model-input redaction pass shared by plain, tool-capable, streaming, and repair provider calls.
  • Preserve surrounding context and raw durable data while replacing detected values with [REDACTED_SECRET].
  • Keep prompt-injection handling separate: untrusted memory remains enveloped and an injection-bearing snippet is omitted without rejecting the turn.

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

Linked Issue

None.

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --benches --tests --examples --all-features -- -D warnings
  • cargo test -p ironclaw_safety --no-fail-fast (308 passed)
  • cargo test -p ironclaw_loop_contracts --no-fail-fast
  • cargo test -p ironclaw_turns --test agent_loop_host_contract --no-fail-fast (87 passed)
  • cargo test -p ironclaw_host_runtime --test memory_prompt_context --no-fail-fast (18 passed)
  • cargo test -p ironclaw_loop_host --test llm_gateway --no-fail-fast (92 passed)
  • cargo test -p ironclaw_loop_host --lib provider_bound_redaction_covers_stop_sequences --no-fail-fast
  • cargo test -p ironclaw_architecture_tests --no-fail-fast
  • RUST_MIN_STACK=16777216 cargo test -p ironclaw_integration_tests --test reborn_integration_golden_payload --no-fail-fast (21 passed)
  • cargo test -p <owning-crate> --features integration: Not applicable; no database-backed behavior changed.
  • Manual testing: exact-head Railway preview validated a neutral JSON attachment through upload, agent recovery, and refresh persistence.

Test Strategy

User behavior: Given recovered text containing security vocabulary, host paths, credential-shaped values, or prompt-injection markers, when IronClaw reconstructs and dispatches a model request, benign context remains usable, detected credential values are placeholders, injection-bearing memory snippets are isolated, and the turn continues.

Risk areas:

  • Model behavior
  • Browser
  • Side effect
  • Persistence
  • Security or permissions
  • External provider
  • Cross-component behavior

Tests added or updated:

  • Unit/contract: known credential formats, labeled weak values, Basic/Bearer authorization, UTF-8 boundaries, idempotency, benign near misses, large-input regex budget, structural prompt limits, and provider stop sequences.
  • Caller contract: credential-bearing runtime, skill, instruction, and capability content no longer aborts prompt construction.
  • Backend/runtime: production memory admission keeps security prose and paths, redacts credential values, drops injection-bearing snippets, and preserves model-content budgets.
  • Provider boundary: recording providers prove secrets are absent from every message role, textual reasoning/content parts, tool descriptions, JSON schema strings/keys, prior tool arguments, and provider-generated repair prompts.
  • Reborn integration: the production-wired durable file-tool path proves structured credential values are absent from persisted tool-result references and every captured model request while benign fields survive.
  • Recorded fixture: Not applicable; no model-selection behavior or nondeterministic output is asserted.
  • Browser E2E: exact-head Railway preview upload and neutral recovery flow passed, including persistence after refresh.
  • Live canary: exact-head Railway preview used a fresh synthetic credential value; the marker, source path, message, and benign secretary field survived, while the password rendered as [REDACTED_SECRET] and the canary was absent.

What the tests prove: detected secrets do not reach provider-visible prompt content, while false positives no longer reject a whole turn or remove unrelated context. Prompt-injection containment remains independent of secret handling.

Security Impact

The model-input boundary now scans immediately before provider dispatch, after all prompt assembly and before prompt-cache hashing. It covers plain/tool/streaming/repair paths; message content across roles; text content parts; plain reasoning and summaries; tool-call argument string keys/values and parse errors; tool descriptions and JSON schema strings/keys; and stop sequences. It logs only aggregate redaction counts, never detected values.

Opaque protocol material that requires exact replay—encrypted/redacted reasoning payloads, signatures, tool-call IDs/names, route/model identifiers, and provider metadata—is not treated as prompt text. Managed credentials remain host-side and continue to use mediated injection rather than prompt construction.

This guarantees redaction for formats recognized by the existing leak detector plus labeled weak values such as password: letmein; it is not a cryptographic proof that arbitrary unlabeled prose cannot contain a secret. The strongest invariant remains: secrets known to IronClaw must stay in the secret store and never be assembled into model input. The final scan is defense in depth for untrusted text and accidental leakage.

Warn-only high-entropy hex findings remain visible unless they are credential-labeled, so ordinary SHA-256 fingerprints do not mutate prompts or prompt-cache identity.

Prompt injection is intentionally not conflated with secret detection. Existing trust envelopes and injection checks still isolate offending untrusted snippets. SkillSpector-style skill trust scanning and NeMo Guardrails-style broader input/output policy remain follow-up layers rather than dependencies of this recovery hotfix.

Reborn Trust-Boundary Checklist

  • Public policy/evidence/trust-bearing types: the new public safety transform represents model-visible redaction results without exposing findings.
  • Untrusted content enters prompts through existing structural validation and trust envelopes, then receives source-independent redaction at provider dispatch.
  • Hashes declare purpose; prompt-cache hashing now observes the redacted system prompt.
  • New/changed status, exit, policy, runtime, or error variants: none.
  • Security/durability serde(default) fields: none changed.
  • Queues/maps/buffers/counters: no durability or concurrency semantics changed.
  • Driver/operator-visible errors: credential content no longer creates a policy-denied turn; structural errors are unchanged.
  • Sandbox/native/host names: no trust-boundary names changed.

Database Impact

None. Raw stored LLM/thread/memory data is retained; redaction applies to the transient model-facing view.

Blast Radius

Prompt construction contracts, production memory admission, and all loop-host provider request shapes. A regression could leak a recognized secret, corrupt opaque replay data, or reintroduce thread-wide rejection. Caller-level provider captures, memory integration tests, structural contract tests, all-features clippy, and architecture ratchets cover those directions.

Compatibility and Rollback

No wire or storage migration. Removing the unused base64 dependency from ironclaw_loop_contracts shrinks the contract layer. Roll back by reverting this PR; stored data requires no repair. During rollback, the prior credential denylist behavior—and its false-positive thread failures—would return.

Follow-up

  • Evaluate SkillSpector in the skill install/update trust pipeline, not on every runtime text field.
  • Prototype NeMo Guardrails or an equivalent classifier as an observable, shadow-mode prompt-injection layer before considering enforcement.
  • Add canary metrics for redaction frequency and snippet-level injection omission without logging source text.

Review track: C (security/runtime/DB/CI)

Prior exact-head Railway QA (269c020)

The first live attempt exposed a second path: after direct and JSON-encoded results were redacted, the agent retried through shell with od -c. Its character-separated output (p a s s w o r d) bypassed label matching even though the model could reconstruct the value. The final fix decodes only offset-prefixed character dumps for detection and replaces the encoded field when the reconstructed text contains a credential assignment. A benign Secretary of the Treasury dump remains unchanged.

Validation on 269c0206f8bf26ac233cd62a4e92cab9e19bba5b:

  • cargo test -p ironclaw_safety --no-fail-fast (308 passed)
  • Provider-boundary character-dump regression in llm_gateway
  • Clippy for ironclaw_safety and the llm_gateway test target with -D warnings
  • reborn_dependency_boundaries (41 passed)
  • Repository pre-commit safety hook
  • Railway deploy check and CodeRabbit
  • Neutral browser QA with a fresh synthetic canary; benign values survived, password was [REDACTED_SECRET], canary absent
  • Refresh/replay preserved the same redacted transcript

No real credential was used. The synthetic QA task and local fixture were deleted after verification.

Final exact-head Railway QA (362966a)

  • Commit-scoped Railway deployment succeeded for 362966a2417eaa2babb7d28cbabaa9da7595d7a4.
  • Neutral browser upload exercised the real JSON attachment and model/tool recovery path using only: Read the attached JSON and report every field and its value.
  • Marker, source path, benign message, and secretary value survived; password rendered as [REDACTED_SECRET]; the exact synthetic canary and distinctive tail were absent.
  • A full refresh restored the completed durable task with the same safe result and no running/interrupted state.
  • Temporary Railway task, local fixture, credential variable, and browser tabs were cleaned up.
  • Canonical evidence: fix(safety): redact model-bound secrets without rejecting turns #7509 (comment)

Final review regressions on this head cover propagation of sensitive schema context through nested composition/applicator keywords, credential redaction in URL fragments while benign state remains visible, and the corrected memory-path expectation. CodeRabbit reported no new actionable comments.

@railway-app

railway-app Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-7509 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Aug 12, 2026 at 2:09 pm

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 11, 2026 16:17 Destroyed
@github-actions github-actions Bot added scope: docs Documentation size: XL 500+ changed lines risk: low Changes to docs, tests, or low-risk modules labels Aug 11, 2026
@coderabbitai

coderabbitai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Security
    • Secrets are automatically redacted from model-bound messages, tools, schemas, repair prompts, URLs, attachments, previews, and tool results.
    • Nested and structured content is sanitized while preserving safe context, ordinary security terminology, paths, and data URLs.
    • Redaction occurs at the model gateway, with safeguards for malformed or oversized content.
  • Reliability
    • Untrusted memory content receives structural validation before inclusion.
    • Unsafe injection content is excluded without disrupting valid runs.
  • Documentation
    • Updated guidance explains model-input redaction and structural validation behavior.

Walkthrough

The change removes prompt-content secret denylists, preserves structural validation, and adds deterministic redaction at memory admission, model-context projection, and provider dispatch.

Changes

Prompt security boundaries

Layer / File(s) Summary
Model-input redaction primitive
crates/substrates/ironclaw_safety/src/*, crates/substrates/ironclaw_safety/README.md
Adds deterministic text, URL, JSON, path, and character-dump redaction with leak classification and regression tests.
Structural validation and memory admission
crates/contracts/ironclaw_loop_contracts/src/*, crates/kernel/ironclaw_host_runtime/src/*, crates/app/ironclaw_architecture_tests/tests/*
Replaces content denylists with structural validation. Memory snippets redact credentials before truncation and preserve safe prose and paths.
Prompt construction and channel context
crates/kernel/ironclaw_turns/tests/*, crates/loop/ironclaw_loop_host/src/lib.rs, crates/loop/ironclaw_loop_host/tests/*
Preserves credential-shaped content, security vocabulary, and host paths for gateway redaction.
Provider-bound recursive redaction
crates/loop/ironclaw_loop_host/src/model_gateway*, crates/loop/ironclaw_loop_host/tests/llm_gateway.rs
Redacts provider messages, tools, schemas, stop sequences, image URLs, cache inputs, and repair prompts.
Result previews and attachment projections
crates/contracts/ironclaw_host_api/*, crates/domains/ironclaw_threads/*, tests/integration/*
Redacts credentials in structured previews, durable tool results, and extracted attachment content while preserving benign content.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PromptBuilder
  participant MemoryContext
  participant ModelGateway
  participant Provider
  PromptBuilder->>MemoryContext: materialize structurally valid content
  MemoryContext->>ModelGateway: pass content after memory redaction
  ModelGateway->>Provider: dispatch recursively redacted request
Loading
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses Conventional Commits syntax and accurately summarizes provider-bound secret redaction.
Description check ✅ Passed The description covers the required sections, security impact, validation, test strategy, blast radius, rollback, and review track.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

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

@serrrfirat

Copy link
Copy Markdown
Collaborator Author

Temporarily closing and reopening to retrigger the Railway preview after the superseded fork PR cleanup cancelled the first deployment.

@serrrfirat serrrfirat closed this Aug 11, 2026
@ironloopai

ironloopai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

🧭 IronLoop Run · Review

This comment updates in place as the Run moves through its stages.

⬛ Final result · Stopped

🟨 Queued → 🟦 Working → ⬛ Stopped

Automatic trigger · stopped after <1s

IronLoop stopped because the pull request was closed while this Run was active.

Run details

Run: bd68a512-b35f-4cd4-9c70-7c70d4ea8c5c
Base: main at 2d6ec0a
Head: codex/fix-thread-recovery-prompt-denylist at 6eff8b9
Created: 2026-08-11 16:18 UTC
Updated: 2026-08-11 16:18 UTC

@serrrfirat serrrfirat reopened this Aug 11, 2026
@ironloopai

ironloopai Bot commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

🧭 IronLoop Run · Review

This comment updates in place as the Run moves through its stages.

🟩 Final result · Completed

🟨 Queued → 🟦 Working → 🟦 Posting results → 🟩 Completed

Automatic trigger · attempt 1 of 3 · completed in 18m 53s

IronLoop completed the review and posted it to GitHub.

🔗 Result

Open submitted review →

Run details

Run: 7e19a04a-f33a-4dde-9a4c-15fb9e04ee08
Base: main at 2d6ec0a
Head: codex/fix-thread-recovery-prompt-denylist at 6eff8b9
Created: 2026-08-11 16:19 UTC
Updated: 2026-08-11 16:38 UTC

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/kernel/ironclaw_host_runtime/src/memory_context.rs (1)

294-305: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

.ok() drops the validation error with no diagnostic.

from_untrusted_memory returns AgentLoopHostError, and .ok() discards it. The caller at line 148 then skips the snippet silently. Every sibling drop path in this file logs first: sanitize_context_snippet at line 229 and admit_lane at line 180 both emit tracing::debug!. A memory snippet that fails structural validation now disappears with no signal.

The repo rule is explicit: fail loud, or name the fallback. Log the rejection at debug! (not info!) and add the // silent-ok: marker.

🔍 Proposed fix
-    LoopContextSnippet::from_untrusted_memory(snippet_ref, snippet.text).ok()
+    // silent-ok: a structurally invalid memory snippet degrades to "not admitted"
+    // rather than failing the turn; the drop is recorded below.
+    LoopContextSnippet::from_untrusted_memory(snippet_ref, snippet.text)
+        .inspect_err(|error| {
+            tracing::debug!(
+                error_kind = ?error.kind,
+                error_safe_summary = %error.safe_summary,
+                "dropping memory snippet rejected by structural prompt validation"
+            );
+        })
+        .ok()

As per coding guidelines: "Do not use .unwrap_or_default() on Result, .ok()? ... justified fallbacks must include an inline // silent-ok: <reason> comment naming the operation."

🤖 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/kernel/ironclaw_host_runtime/src/memory_context.rs` around lines 294 -
305, Update to_loop_context_snippet so a failed
LoopContextSnippet::from_untrusted_memory validation is logged with
tracing::debug! before returning None, including the rejection error and
relevant snippet context. Add the required inline // silent-ok: marker
documenting the intentional fallback, while preserving successful conversions.

Sources: Coding guidelines, Path instructions

🤖 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/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs`:
- Around line 730-735: Update the ratchet entry for ironclaw_loop_contracts to
describe the actual production growth: validation now checks structural limits
and control characters only, with Basic-auth samples limited to tests. Set the
ceiling to 13,495 to remain within the 400-line tolerance of the 13,095
production count, and append “Count read from this test's own failure message.”

In `@crates/contracts/ironclaw_loop_contracts/src/prompt_text.rs`:
- Around line 155-157: Update the documentation comment above
validate_prompt_text_with_diagnostics to remove the obsolete claim that a trust
gate relaxes credential-shaped value checks. Describe structural
control-character rejection across all surfaces and direct readers to the
provider-bound redaction boundary for credential handling.

In `@crates/kernel/ironclaw_turns/tests/agent_loop_host_contract.rs`:
- Around line 1542-1559: Align the trust-level terminology in both affected test
names and fixtures: either rename the tests to indicate Installed skills, or
change their skill_instruction_request fixtures to SkillTrustLevel::Untrusted if
that variant exists. Ensure the names accurately describe the trust level
actually exercised.

In `@crates/loop/ironclaw_loop_host/src/model_gateway.rs`:
- Around line 1653-1847: Move the self-contained redaction helpers, including
redact_completion_request, redact_tool_completion_request, redact_json_object,
and related functions, into a new model_gateway/redaction.rs submodule. Import
only the required ironclaw_llm types, serde_json collections, and
redact_model_input_text, then update model_gateway to reference the module
without changing dispatch behavior. Place or relocate focused unit tests for the
JSON-schema key remapping there.
- Around line 1653-1662: Measure the redaction cost in redact_completion_request
and redact_tool_completion_request using the existing CONTEXT_SHADOW_TARGET
shadow-measurement path, then eliminate repeated scans of unchanged replayed
content by caching redaction results keyed by immutable content_ref and/or
redacting tool schemas once at the capability-surface seam. Preserve redaction
behavior while ensuring each unchanged message and tool definition is processed
only once across provider dispatches.
- Around line 1678-1697: Update redact_chat_message to process
ContentPart::ImageUrl image_url.url before provider dispatch, using URL-aware
redaction that removes query credentials while preserving data: payloads; do not
rely on redact_string alone. Include the redacted URL changes in the returned
count and add a regression test for the recording provider covering
credential-bearing image URLs.

In `@crates/loop/ironclaw_loop_host/tests/llm_gateway.rs`:
- Around line 225-286: Extend
gateway_redacts_tool_descriptions_and_schema_strings_before_dispatch to exercise
prompt-cache signatures: submit two otherwise identical requests whose only
difference is a secret removed by redaction, then assert their recorded
system_prompt_cache_signature values are equal. Use the existing
provider/request recording symbols and preserve the current dispatch redaction
assertions.

In `@crates/substrates/ironclaw_safety/src/model_input_redaction.rs`:
- Line 82: Decouple the redaction filter in the model-input redaction flow from
the detector-internal "high_entropy_hex" literal by using a typed LeakDetector
accessor or enum-based pattern classification. Preserve the behavior that
high-entropy hexadecimal findings, including SHA-256 fingerprints, are excluded
from redaction; add detector-crate coverage that locks this classification or
name contract.

---

Outside diff comments:
In `@crates/kernel/ironclaw_host_runtime/src/memory_context.rs`:
- Around line 294-305: Update to_loop_context_snippet so a failed
LoopContextSnippet::from_untrusted_memory validation is logged with
tracing::debug! before returning None, including the rejection error and
relevant snippet context. Add the required inline // silent-ok: marker
documenting the intentional fallback, while preserving successful conversions.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2f8e73f6-346b-4a0e-90be-a7db34ad3a9e

📥 Commits

Reviewing files that changed from the base of the PR and between 2d6ec0a and 6eff8b9.

📒 Files selected for processing (16)
  • crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs
  • crates/app/ironclaw_architecture_tests/tests/reborn_extension_specificity.rs
  • crates/contracts/ironclaw_loop_contracts/src/host/context.rs
  • crates/contracts/ironclaw_loop_contracts/src/instruction_bundle.rs
  • crates/contracts/ironclaw_loop_contracts/src/prompt_text.rs
  • crates/contracts/ironclaw_loop_contracts/src/runtime_context/tests.rs
  • crates/kernel/ironclaw_host_runtime/src/memory_context.rs
  • crates/kernel/ironclaw_host_runtime/tests/memory_prompt_context.rs
  • crates/kernel/ironclaw_turns/tests/agent_loop_host_contract.rs
  • crates/loop/ironclaw_loop_host/src/lib.rs
  • crates/loop/ironclaw_loop_host/src/model_gateway.rs
  • crates/loop/ironclaw_loop_host/tests/llm_gateway.rs
  • crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs
  • crates/substrates/ironclaw_safety/README.md
  • crates/substrates/ironclaw_safety/src/lib.rs
  • crates/substrates/ironclaw_safety/src/model_input_redaction.rs
💤 Files with no reviewable changes (1)
  • crates/app/ironclaw_architecture_tests/tests/reborn_extension_specificity.rs

Comment thread crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs Outdated
Comment thread crates/contracts/ironclaw_loop_contracts/src/prompt_text.rs Outdated
Comment thread crates/kernel/ironclaw_turns/tests/agent_loop_host_contract.rs Outdated
Comment thread crates/loop/ironclaw_loop_host/src/model_gateway.rs Outdated
Comment thread crates/loop/ironclaw_loop_host/src/model_gateway.rs Outdated
Comment thread crates/loop/ironclaw_loop_host/src/model_gateway.rs Outdated
Comment thread crates/loop/ironclaw_loop_host/tests/llm_gateway.rs
Comment thread crates/substrates/ironclaw_safety/src/model_input_redaction.rs Outdated
@serrrfirat

serrrfirat commented Aug 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

Railway preview QA — FAIL

Correction: the previous PASS evidence used a prescriptive prompt that told the agent not to reveal the credential. That did not prove automatic redaction. This comment replaces that evidence with two neutral tests.

Given / When / Then matrix

Acceptance Intended contract Actual contract exercised Observed evidence Result
Required A credential-shaped attachment field must be redacted before reaching the model, without relying on user instructions. Uploaded a synthetic JSON attachment through the live authenticated WebUI and asked only: “Read the attached JSON and report every field and its value.” The agent reported all four fields and included the complete synthetic password value in its response. It visually labeled the value as redacted, but displayed the original value beside that label. FAIL
Required The automatic-redaction failure must be reproducible rather than a one-off model response. Started a fresh task with an independent JSON fixture and different synthetic password value; asked only: “List the JSON fields and values.” The agent again reported the complete synthetic password value. No redaction or secrecy instruction was present. FAIL
Required Benign attachment context must remain usable and the task must not be rejected. Both neutral attachment tasks used the production upload-to-chat path on the exact deployed head. Both tasks completed and correctly reported the marker, source path, message, and field count. PASS
Required The resulting task must remain recoverable after refresh. Reloaded the independent reproduction thread after completion. The attachment and full response rendered again. The synthetic password disclosure also remained visible after refresh. PASS for recovery; disclosure remains a failure
Supplemental Preview authentication and ordinary dispatch are healthy. Gateway-token login through the visible preview form and two live attachment turns. Authentication succeeded and both turns completed. PASS

Status derivation

  • Required passed: 2
  • Required failed: 1 contract, reproduced twice
  • Required blocked/not executed: 0
  • Overall: FAIL because the automatic provider-bound secret-redaction claim was contradicted by the intended live attachment path.

Exact regression result

The PR fixes whole-turn rejection: credential-shaped attachments remain usable and their benign context reaches the assistant. However, the stronger security claim is not satisfied for this live attachment path. With neutral prompts, the assistant received and echoed the complete synthetic password value in two independent tasks. The second disclosure persisted after refresh.

Excluded prior evidence

The earlier instruction “do not reveal any credential value” is excluded from the automatic-redaction acceptance result because it tested model instruction-following rather than the boundary guarantee.

Remaining risk

The observed behavior indicates that at least one attachment/document-read path can deliver a credential-shaped value to the model despite the final model-bound scan. The exact missing boundary requires code/log diagnosis; this browser run does not infer which internal hop bypassed redaction.

Cleanup

Cleanup complete: both synthetic tasks were deleted, both local fixtures were removed, and the browser test tab was finalized.

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 IronLoop review

Found two high-severity security defects in the provider-bound redaction flow.

Findings: 🔴 High 2

🔴 High · Redact values paired with sensitive JSON keys

Inline on crates/loop/ironclaw_loop_host/src/model_gateway.rs:1722. See the inline comment for details.

🔴 High · Keep raw host paths out of provider prompts

Inline on crates/contracts/ironclaw_loop_contracts/src/prompt_text.rs:192. See the inline comment for details.

Validation

  • ✅ Focused safety redaction tests — 6 targeted model-input redaction tests passed.
  • ✅ Focused memory-boundary test — The targeted changed memory prompt-boundary test passed (1 test).
  • ⚪ Broad validation — Not run. Not run; static call-path analysis and focused checks were sufficient for this review.
Review details
  • Run: 7e19a04a-f33a-4dde-9a4c-15fb9e04ee08
  • Workflow: Review
  • Attempts: 1

Comment thread crates/loop/ironclaw_loop_host/src/model_gateway.rs Outdated
Comment thread crates/contracts/ironclaw_loop_contracts/src/prompt_text.rs
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 11, 2026 19:33 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/loop/ironclaw_loop_host/tests/llm_gateway.rs (1)

177-226: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Add an attachment-to-provider regression test.

gateway_redacts_every_message_role... sets image_parts: Vec::new() and bypasses attachment ingress. The existing attachment test stops at RecordingGateway and asserts raw image bytes, not the provider request. This violates the AGENTS.md “Test through the caller” invariant. Drive the attachment path through LlmProviderModelGateway and assert [REDACTED_SECRET] without the original value before claiming provider-bound redaction.

🤖 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/loop/ironclaw_loop_host/tests/llm_gateway.rs` around lines 177 - 226,
The current regression test only verifies text redaction and does not exercise
attachment ingress through the provider gateway. Extend the test around
gateway.stream_model and the model_request setup to include an attachment
containing a secret, route it through LlmProviderModelGateway to the provider,
and assert the provider-bound request contains [REDACTED_SECRET] while excluding
the original secret value.

Source: Path instructions

🤖 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/substrates/ironclaw_safety/src/model_input_redaction.rs`:
- Around line 15-18: The credential-redaction regex in the model-input redaction
logic currently stops at whitespace and delimiters inside quoted values. Update
the relevant pattern and matching logic to support complete single-, double-,
and backtick-quoted values, including escaped quotes, for both structured
credentials and Authorization values; preserve unquoted matching behavior. Add
regression tests covering spaces, commas, semicolons, and escaped quotes, and
verify the full credential is redacted according to the documented safety
invariant.

---

Outside diff comments:
In `@crates/loop/ironclaw_loop_host/tests/llm_gateway.rs`:
- Around line 177-226: The current regression test only verifies text redaction
and does not exercise attachment ingress through the provider gateway. Extend
the test around gateway.stream_model and the model_request setup to include an
attachment containing a secret, route it through LlmProviderModelGateway to the
provider, and assert the provider-bound request contains [REDACTED_SECRET] while
excluding the original secret value.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c2d9e54d-7f2c-4925-90ec-966c6b339349

📥 Commits

Reviewing files that changed from the base of the PR and between 6eff8b9 and 570abd5.

📒 Files selected for processing (2)
  • crates/loop/ironclaw_loop_host/tests/llm_gateway.rs
  • crates/substrates/ironclaw_safety/src/model_input_redaction.rs

Comment thread crates/substrates/ironclaw_safety/src/model_input_redaction.rs
…very-prompt-denylist

# Conflicts:
#	crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 12, 2026 10:42 Destroyed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/loop/ironclaw_loop_host/src/lib.rs (1)

2990-2997: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Consume the complete tool-result group.

load_task_pinned_context_window removes only one ToolResultReference after an assistant. Multiple distinct results can follow one assistant. With [Assistant, Ref1, Ref2], Ref2 remains model-visible and recent_window_truncation stops at Ref1. Remove the full consecutive group and extend task_pin_evicts_complete_tool_exchange_at_a_compactable_boundary with two references. This follows the repository’s caller-level test-through-the-caller invariant.

🤖 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/loop/ironclaw_loop_host/src/lib.rs` around lines 2990 - 2997, Update
load_task_pinned_context_window to remove the entire consecutive
ToolResultReference group following a displaced Assistant, not just the first
reference, so later references are not model-visible and truncation reaches the
group boundary. Extend
task_pin_evicts_complete_tool_exchange_at_a_compactable_boundary with a case
containing two references and verify both are consumed through the caller-level
behavior.
🤖 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.

Outside diff comments:
In `@crates/loop/ironclaw_loop_host/src/lib.rs`:
- Around line 2990-2997: Update load_task_pinned_context_window to remove the
entire consecutive ToolResultReference group following a displaced Assistant,
not just the first reference, so later references are not model-visible and
truncation reaches the group boundary. Extend
task_pin_evicts_complete_tool_exchange_at_a_compactable_boundary with a case
containing two references and verify both are consumed through the caller-level
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 375d06c0-5948-4d98-b525-7faf4b22f7cb

📥 Commits

Reviewing files that changed from the base of the PR and between 362966a and 63dfe88.

📒 Files selected for processing (4)
  • crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs
  • crates/contracts/ironclaw_loop_contracts/src/host/context.rs
  • crates/loop/ironclaw_loop_host/src/lib.rs
  • crates/loop/ironclaw_loop_host/tests/thread_loop_host_contract.rs

…very-prompt-denylist

# Conflicts:
#	crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 12, 2026 12:04 Destroyed
@serrrfirat
serrrfirat enabled auto-merge August 12, 2026 12:30
BenKurrek
BenKurrek previously approved these changes Aug 12, 2026
@serrrfirat
serrrfirat added this pull request to the merge queue Aug 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 12, 2026
…very-prompt-denylist

# Conflicts:
#	crates/app/ironclaw_architecture_tests/tests/reborn_dependency_boundaries.rs
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 12, 2026 12:56 Destroyed
@serrrfirat
serrrfirat enabled auto-merge August 12, 2026 13:33
BenKurrek
BenKurrek previously approved these changes Aug 12, 2026
@serrrfirat
serrrfirat added this pull request to the merge queue Aug 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/integration/extension_visibility.rs`:
- Around line 95-102: The system-prompt assertion in the extension visibility
test is not specific to the local description and can pass using the verified
description. Update the local description fixture and its corresponding
assertion around assert_system_prompt_contains to include a unique local-only
marker, then assert that marker in the local system-prompt projection while
preserving the existing verified-source checks.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 06c1729e-a4e6-4f59-b777-bb8df51462f4

📥 Commits

Reviewing files that changed from the base of the PR and between 36e5873 and c4ec257.

📒 Files selected for processing (3)
  • tests/CLAUDE.md
  • tests/integration/extension_visibility.rs
  • tests/integration/support/harness/profiles/extension.rs

Comment on lines +95 to +102
.assert_model_tool_description_contains(
"verifiedprompt__invoke",
AUTH_VOCABULARY_DESCRIPTION,
)
.await
.expect("verified catalog description reaches the model intact, including Bearer");
harness
.assert_system_prompt_contains(PROMPT_DENIAL_DESCRIPTION)
.assert_system_prompt_contains(AUTH_VOCABULARY_DESCRIPTION)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the local system-prompt assertion source-specific.

assert_system_prompt_contains(AUTH_VOCABULARY_DESCRIPTION) at Line 102 can be satisfied by the verified description. The local assertion at Lines 118-120 checks only localprompt.unsafe. The test can therefore pass if the local description is still omitted from the system prompt. Add a local-only marker and assert it in the local system-prompt projection.

Also applies to: 114-120

🤖 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 `@tests/integration/extension_visibility.rs` around lines 95 - 102, The
system-prompt assertion in the extension visibility test is not specific to the
local description and can pass using the verified description. Update the local
description fixture and its corresponding assertion around
assert_system_prompt_contains to include a unique local-only marker, then assert
that marker in the local system-prompt projection while preserving the existing
verified-source checks.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-7509 August 12, 2026 14:09 Destroyed
@serrrfirat
serrrfirat added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit ba5e07c Aug 12, 2026
53 checks passed
@serrrfirat
serrrfirat deleted the codex/fix-thread-recovery-prompt-denylist branch August 12, 2026 14:49
BenKurrek added a commit that referenced this pull request Aug 12, 2026
…wn-ratchet

Second refresh fold of the day (#7509 + #7365). Conflicts were again the
composition bookkeeping pair; resolved by keeping both dated chains and
taking the merged tree's own measurement:

- composition-budget.toml + reborn_restructure_baselines.rs: main's #7365
  re-ratcheted DOWN 41810 -> 41533 (memory-save guidance evicted to the
  memory-native package); this branch adds no composition code, and the
  merged tree measures 41533 LOC / 831 Arc<dyn> exactly, so main's banked
  eviction stands and the arc_dyn re-equalization from the previous fold
  carries through. Verified with the gate's --print.
- Main's three SIZE_CEILINGS bumps (extension_contracts 7947, host_api
  19086, loop_contracts 13524) auto-merged; the ceiling gate passes on the
  merged tree with the windows intact.

Verified green: reborn_dependency_boundaries 42/42 (incl. the window-edge
fixture), reborn_restructure_baselines, composition-budget gate, clippy
clean, fmt clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenKurrek added a commit that referenced this pull request Aug 12, 2026
…ates re-measured on the merged tree

Conflicts were the three measurement files only. Ceilings re-measured, not
summed (composition 41780 LOC / 838 governed Arc<dyn> sites; contracts-tier
ceilings verified passing under the union records).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
l3ocifer pushed a commit to l3ocifer/frick-ironclaw that referenced this pull request Sep 3, 2026
…ai#7509)

* fix(loop): allow security prose in recovered context

* test(turns): align prompt safety contract coverage

* fix(loop): address review feedback on prompt recovery (nearai#7434)

* fix(loop): reject filler-separated credentials (nearai#7434)

* fix(safety): redact model-bound secrets without rejecting turns

* fix(safety): preserve non-secret sha256 fingerprints

* test(safety): align channel context with gateway redaction

* fix(gateway): address coderabbit review — preserve redacted JSON shape (nearai#7434)

* fix(safety): redact quoted structured credentials

* fix(safety): scan encoded tool result content

* fix(safety): redact structured credential values

* fix(safety): redact character-dump credentials

* fix(safety): close provider-bound redaction gaps (nearai#7509)

* fix(safety): close structured redaction review gaps (nearai#7509)

* fix(safety): redact nested schema and URL fragment secrets (nearai#7509)

* test(integration): align prompt trust expectation (nearai#7509)

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-7509 — c4ec2573 Deployed Aug 12, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants