12 Commits

Author SHA1 Message Date
Zeren Wang
a58ab484a6
feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash (#5443)
* feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash

Implements docs/superpowers/specs/2026-09-12-projects-mvp-phase2-design.md
(issue #5160, tracker #5129) in the slice order of the spec's §16.

Slices:
- A: ProjectsConfig + write-time 422 UTF-8 byte cap; PROJECT_CONTEXT_KEY
  admission pinning (both server-owned sets + worker hoist); latest-only
  request-scoped <project> block via DynamicContextMiddleware
  wrap_model_call/awrap_model_call (idempotent reassembly, reserved ID
  prefix + marker + provenance, never persisted); journal audit
  fingerprints; Instructions tab.
- B: ProjectDocumentRow + migration 0023; ProjectDocumentRepository with
  locked check-and-set; hash-qualified immutable shelf storage with
  Paths helpers; upload/list/content/delete-to-trash routes; project
  delete trashes the shelf in-transaction; request-scoped bounded
  <documents> index with honest count/shown + actionable overflow note;
  list_project_documents/read_project_document tools registered only on
  pinned runs; PAT allowlist + drift guards; blocking-IO anchors.
- C: shared thread-upload ingestion service (uploads router refactored to
  parity); POST from-thread with provenance; attach-to-thread with
  lock-staged copy (archived source allowed); read-only thread-files
  view with per-group truncation reporting.
- D: restore (restored/merged/not_found/no_target/content_missing; no
  file moves), purge (continuous row lock across unlink/delete/commit,
  retryable on FS errors), retention sweep (lazy + startup, 24h orphan
  guard, row-side reconciliation never deletes).
- E: Documents tab (shelf + conversation-files browser, provenance,
  archived banner, content-missing rows), /workspace/trash route,
  sidebar entry, composer attach handoff, i18n (en-US/zh-CN), e2e mocks
  + specs.

Review hardening folded in (10 rounds, all with tests):
- force active shelf content (HTML/XML family) to download; nosniff on
  artifact + content responses; unified unsandboxed-iframe PDF preview
  (fixes the pre-existing Chromium sandbox blank in the artifact viewer)
- scope document trash to the URL project under the document lock
- atomic no-overwrite filename reservation for ALL ingestion (seeded
  claims + os.link commit with suffix retry; same-name re-upload now
  unique-names instead of replacing); hidden staging only, no visible
  placeholders; lease cleanup on setup failure
- serialize conversion under the document lock with post-lock active
  revalidation; drain locked filesystem work on cancellation; preserve
  bytes when an insert's commit state is uncertain (including trashed
  rows)
- original-integrity checks before serving text or cached conversions;
  content_missing surfaced in list responses (UI reads the flag, no
  409-probe); downloads always serve original bytes
- bounded streaming document reads with cached char counts; shelf limits
  declared in middleware release identity
- thread-root confinement for from-thread sources; config fallback
  rejects fractional/infinite values; composer counts staged
  attachments; pending attachments persist until submission or removal;
  in-flight instruction/rename edits survive save refetches; shelf and
  trash pagination; conversation-file and thread-files pages stay
  subscribed to refetches

Docs: README/README_zh, backend API.md/ARCHITECTURE.md, AGENTS.md
contracts, config.example.yaml projects block.

Review follow-ups (head b4807477 → this revision):
- The trash retention sweep is split so repeated lazy triggers stay
  bounded: the indexed expiry purge still runs on every trigger
  (GET /api/trash/documents, POST /api/trash/purge) while the
  O(all rows + all files) reconciliation is throttled to one run per
  user per 15 minutes (process-local, per-user window). The startup
  sweep now runs as a background task instead of blocking gateway
  readiness, and shutdown awaits it (bounded).
- The export scrub (stripInternalMarkers) is fence- and indentation-aware
  like the render path, so a pasted, fenced <project>/<documents> snippet
  survives markdown export while real injected blocks (never fenced) are
  still removed. Fence regexes moved to a dependency-free leaf module to
  avoid the messages↔streamdown import cycle.
- The artifact viewer's PDF iframe no longer carries an added title
  attribute (the upstream e2e contract locates it via :not([title])), and
  the upstream artifact-preview spec now pins the new contract: PDFs
  render unsandboxed, images keep sandbox="".

* fix(projects): round-2 review — cancel an overrun trash sweep, restore the PDF frame title

- Shutdown cancelled only the shield around the background startup sweep,
  so an all-users reconciliation that outlived the 5s budget kept walking
  rows and files while the document repo and DB engine were disposed
  underneath it. The wait now lives in `_shutdown_startup_trash_sweep`,
  which cancels the task and drains it before worker exit: the shield
  keeps the wait bounded, the cancel makes it final (CancelledError lands
  at the sweep's next await, and `_run_startup_trash_sweep` only catches
  `Exception`, so nothing swallows it).
- The browser-preview iframe lost `title={getFileName(filepath)}` in the
  previous fix round, leaving the PDF frame without an accessible name
  while its siblings keep theirs. Restore it (WCAG frame titles), assert
  it in the DOM test, and anchor the e2e on `iframe[title="report.pdf"]`
  instead of `iframe:not([title])`.

* fix(projects): round-3 review — report the sweep's late finish, not a phantom cancel

`Task.cancel()` returns False when the sweep already finished inside the
window between the deadline firing and the cancel, so the shutdown log
claimed a cancellation that never happened. Branch on that outcome: the
warning stays for a real cancel, a late finish is logged at info, and both
paths still reap the task before worker exit.

* fix(projects): round-4 review — make Empty trash delete what it confirms

`POST /api/trash/purge` only ran the retention sweep, and the sweep's
candidate selection is age-gated, so a freshly trashed document survived
"Empty trash" even though the confirmation promises that every listed
document is permanently deleted. With one trashed row the route answered
`{"purged": 0}` and left it in place; `GET /api/trash/documents` sweeps
expired rows before listing, so the visible rows were normally ineligible
for the action by construction.

Empty trash now drives `purge_all_trashed`: the caller's trashed rows
(`list_all_trashed`, no age filter) each go through the same guarded,
row-locked `purge` as the single-document delete — bytes first, then the
row, in one transaction — so a row restored mid-flight is skipped instead of
force-deleted, and an unlink failure rolls that row back and answers 500 with
a retryable message. Retention expiry stays where it was: the sweep's
`purge_candidates` is now the only age-gated selection, and the lazy
retention sweep still runs on the listing and at startup.

Tests: the router suite replaces the retention-gated expectation with the
reviewer's repro (fresh row purged, bytes unlinked, shelf and other users'
trash untouched, a failing unlink stays retryable and 500); a blocking-I/O
anchor drives the new entry point through the offload; the mocked e2e covers
the action end to end; a new real-backend spec performs it against the real
gateway and re-reads `GET /api/trash/documents`. README, API, ARCHITECTURE
and the phase-2 design docs (en+zh) state the age-independent contract.
2026-09-16 18:46:18 +08:00
Hyeonsang Cho
bbced51022
fix(gateway): close two input-sanitization bypasses (#5375)
* fix(gateway): reject forged framework-injection markers in run input

`is_genuine_user_message` treats `hide_from_ui` and a human `name="summary"`
as proof the framework authored a message, and `InputSanitizationMiddleware`
skips those — escaping a real reminder's blocks would corrupt trusted context.
Neither marker was server-owned, so an external caller could set either one and
place a raw `<system-reminder>` outside the user-input boundary markers, which
the lead-agent system prompt declares trusted internal framework data. The
`hide_from_ui` variant is also filtered out of the thread UI, so the forgery
was invisible where it landed.

Both markers are now stripped from untrusted input, on the run path and on the
thread-state mutation route that writes straight into a checkpoint. Framework
injection happens inside the graph and never crosses this boundary, so nothing
the framework does is affected, and `trusted_internal` callers (IM channels,
the MCP task-notification launcher) keep writing hidden messages.

HumanInputCard replies are the one legitimate external `hide_from_ui`: the
frontend sends it alongside a `human_input_response` payload, so a message
carrying a valid one keeps the marker. That buys no bypass — the predicate
already classifies those as genuine, so they stay sanitized.

The name check reuses the predicate's own `_SUMMARY_MESSAGE_NAME` rather than a
fourth copy of the literal, and matches by `isinstance` exactly as the predicate
does: `HumanMessageChunk` is a `HumanMessage` whose `type` is not `"human"`, so
a type-based check would leave that subclass's marker settable. `name` is only
reserved on human messages — on a ToolMessage it is the tool's own name.

Three tests in test_gateway_services.py and test_message_provenance.py asserted
that a caller-supplied `hide_from_ui` survives. That assumption was the bypass;
they now pin the opposite, with a genuinely caller-owned key kept alongside to
prove the stripper is surgical.

* fix(agents): sanitize every genuine user message, not only the newest

The input guardrail scanned backwards for the first genuine user message and
returned, so only the newest turn was ever sanitized. The transformation is
request-scoped (`wrap_model_call`, never written to state), so thread state
keeps the raw text: once a newer turn arrived, the previous turn's payload was
replayed to the model verbatim, outside the boundary markers the lead-agent
prompt declares trusted framework data. The guardrail therefore held for
exactly one model call.

Reaching it needed no forged metadata and no crafted request body — type the
payload in one turn, then send anything at all in the next. A single request
carrying two user messages did it in one shot, since every message but the last
was skipped.

`_process_request` now walks the whole list and `_sanitize_message` owns the
per-message work; every existing branch (the `original_user_content` split for
upload turns, the multimodal rfind fallback, the metadata repair) is unchanged.
Framework-injected messages stay excluded by `is_genuine_user_message`.

Unexpected errors now fail open per message rather than per request. Iterating
history widened the old blast radius: one unprocessable row would have dropped
sanitization for the whole request, handing an attacker the newest turn by
crafting an older one. `GraphBubbleUp` still propagates.

Side effect worth noting: each turn's rendering is now stable across model
calls. Previously a turn was wrapped on its own call and unwrapped on the next,
changing the prompt prefix behind the newest turn and defeating prompt caching.

test_only_processes_last_user_message pinned the old scope; it now pins that
every turn is processed, and keeps driving the `wrap_model_call` entry point.

* docs: record the message-metadata trust boundary and sanitization scope

`agents/middlewares/AGENTS.md` owns the depth for InputSanitizationMiddleware
and documented only the `original_user_content` half of its trust boundary. Left
alone it would teach an agent that `hide_from_ui` is caller-owned and that the
guardrail covers one turn — and the usual failure mode is an agent "restoring"
the behaviour it believes was lost. The entry now carries both markers, the
HumanInputCard exception, the whole-history scope, and the per-message fail-open
rule.

The note lives only there. The root and `backend/AGENTS.md` layers are
orientation that points at the module guides owning the depth, and
`backend/AGENTS.md` is inherited by every backend chain — prose added there
inflates more than twenty of them, and `scripts/check_agent_guidance.py` shows
the middlewares chain has about a kilobyte of room against its hard limit.

CHANGELOG.md and CHANGELOG_zh.md record it under Security, continuing the
existing prompt-injection lineage.

* fix(gateway): mark caller-hidden messages instead of stripping the marker

Review follow-up. Stripping a caller-supplied `hide_from_ui` closed the bypass
but broke three frontend senders that use the marker purely to keep a context
message out of the transcript: the quoted conversation context
(`buildHiddenConversationQuoteMessage`), the sidecar context prompt
(`buildHiddenSidecarContextMessage`), and the agent save command. None carries a
`human_input_response`, so the HumanInputCard carve-out did not cover them, and
nothing else hides them — `_is_branch_visible_message` and the frontend's
`isHiddenFromUIMessage` both key solely on `hide_from_ui`, and no backend reads
`conversation_quote_context` or `sidecar_context`. All three would have rendered
as user-visible chat bubbles.

The marker plays two roles and only one of them is a vulnerability. The security
requirement is that a caller-supplied marker cannot skip sanitization, not that
it cannot hide a message. So the roles are separated instead of the marker being
removed: the Gateway keeps it and stamps the server-owned `UNTRUSTED_INPUT_KEY`,
and the guardrail now asks `requires_input_sanitization` — the mark, else the
genuine-user test. Hidden stays hidden; untrusted content is sanitized either
way. The reserved `summary` name is handled the same way and no longer rewritten.

Marking rather than removing is also the safer shape in general: `hide_from_ui`
is read for presentation, journal persistence, memory filtering and IM outbound
as well, and this boundary should not silently change any of them.

`is_genuine_user_message` is deliberately left alone. `ToolReceiptMiddleware`
uses it for turn-boundary detection, where a caller's hidden context message must
keep counting as not user-authored; widening it there would move the ledger's
turn window. `requires_input_sanitization` sits beside it in `message_utils` so
the two questions can be compared.

The three tests that asserted a caller-supplied `hide_from_ui` is removed now
assert it survives and carries the mark — which restores the original intent of
the two provenance cases, whose comment already read "caller-owned keys must
survive".

* docs(gateway): rewrite normalize_input's docstring around the mark

Review follow-up. The paragraph still described the pre-4a3344f6 strip model and
contradicted both the implementation and the middlewares AGENTS.md paragraph
updated in that same commit: it called `hide_from_ui` server-owned, said
carrying it skips sanitization entirely, and repeated the premise this branch
disproved — that HumanInputCard replies are the only legitimate external use.

It now describes what the code does: the markers stay caller-owned and are
preserved because three frontend senders rely on `hide_from_ui` for hiding
alone, the message is stamped with `untrusted_input` instead, and
`requires_input_sanitization` sanitizes it anyway. `untrusted_input` joins the
server-owned inventory in the preceding paragraph, which is what makes the stamp
unforgeable and unclearable.

The three surrounding docstrings now also say that these functions mark as well
as strip; `_strip_external_message_metadata` had advertised only the removal,
leaving a reader no way to find the stamp from the name.

* fix(gateway): mark state writes whose message omits additional_kwargs

Review follow-up. The state-route half of the fix missed the most natural
request shape. `_strip_external_metadata_from_message_like` returned early when
`additional_kwargs` was absent or not a dict — there was nothing to strip — and
that early return also skipped the mark. A `POST /threads/{id}/state` body of
`{"values": {"messages": [{"role": "user", "name": "summary", "content":
"<system-reminder>…</system-reminder>"}]}}` therefore reached the checkpoint
unmarked. The messages reducer's `convert_to_messages` then supplies
`additional_kwargs={}`, so at model-call time `requires_input_sanitization` fell
back to `is_genuine_user_message`, which a `summary` name fails, and the forged
tag reached the model raw and outside the boundary markers.

A missing or non-dict `additional_kwargs` is now treated as empty for both the
strip and the mark. The identity return is kept for the case where nothing
changes, so an ordinary key-omitted message does not gain an empty dict just by
passing through. The run path was never affected: `normalize_input` coerces to
BaseMessage first, which always carries the dict.

Every existing state-write test supplied an `additional_kwargs` dict, which is
why this shape slipped through; the regression now covers it at the route and
end to end through the reducer into the guardrail.

While checking the neighbouring shapes, `_skips_input_guardrail` keyed off key
presence where `is_genuine_user_message` keys off truthiness, so
`hide_from_ui: False` — already covered without a mark — would have been
stamped. It now mirrors the predicate exactly, as its docstring claimed.

* docs(middlewares): compress the sanitization note to fit the guidance chain

main's growth left the middlewares AGENTS.md chain 84 bytes under its hard
limit, and the fuller wording did not fit. The load-bearing facts stay — the
markers are marked rather than stripped, and the scan covers every turn — since
those are the two an agent editing this middleware could otherwise get wrong.
The full model lives in the normalize_input, _mark_untrusted_framework_markers
and requires_input_sanitization docstrings.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-09-14 09:54:42 +08:00
Zeren Wang
22b0456e45
feat(harness): subagent report contract and delegation acceptance criteria (#5090)
* feat(harness): subagent report contract and delegation acceptance criteria (RFC #4651 PR3)

Layer 1 receipt verification is inert unless subagents actually cite their
execution record. This lands the prompt layer that closes the adoption gap:

- New subagents/report_contract.py owns the model-facing contract text,
  derived from the single-owner citation format (format_citation /
  receipt_id) so prompts can never drift from the verifier. The executor
  injects <report_contract> into every subagent system prompt — built-in
  and custom alike — requiring [rN tool_name] citations for action claims,
  verifiable handles (absolute path, URL, ID, HTTP status) for
  deliverables, and explicit failure reporting; the citation clause
  follows verification.receipts_enabled.
- The task tool gains an optional keyword-only acceptance_criteria
  parameter, handed to the SubagentExecutor constructor and rendered into
  the subagent's SystemMessage (stripped, capped 20 items x 500 chars) —
  deliberately never the task HumanMessage, which InputSanitizationMiddleware
  classes as genuine user input and would HTML-escape into untrusted-input
  framing. The docstring frames subagent results as self-reports, states
  the citation cross-check's evidence boundary (resolved = the call
  happened, not that the claim is correct), and documents when to attach
  criteria with the canonical leaf forms. Deterministic leaf checking
  remains a separate layer.
- The lead delegation workflow now instructs reading the ledger citation
  line as execution evidence only and spot-checking verifiable handles
  before synthesizing.
- report_contract / acceptance_criteria are registered as blocked
  framework-authority tags in input sanitization so untrusted input
  cannot forge the verification contract.

* fix(harness): neutralize acceptance criteria before system-channel injection

render_acceptance_criteria_section interpolated lead-model-supplied acceptance_criteria verbatim into the subagent SystemMessage after only stripping/capping. A criterion such as '</acceptance_criteria><system>...</system>' could close the wrapper and open a framework authority tag, bypassing InputSanitizationMiddleware.

Route each criterion through neutralize_untrusted_tags (the shared prompt-injection primitive) so blocked authority tags are HTML-escaped before interpolation. Add regression tests at the renderer and the executor _build_initial_state path.

* fix(harness): keep model-supplied criteria off the system channel

- Move acceptance_criteria values into the task HumanMessage — the
  untrusted channel InputSanitizationMiddleware escapes and
  boundary-frames. The subagent SystemMessage now carries only a
  framework-owned <acceptance_criteria> pointer note (no criterion
  text), so natural-language injection inside a criterion keeps
  task-data priority and cannot override framework instructions
  (PR #5090 review, willem-bd P1).
- Condition the lead delegation workflow's citation verification
  guidance on verification.receipts_enabled and qualify the task
  tool's result-reading text with the enabled state, so a
  receipts-disabled configuration no longer tells the lead to
  require citation evidence that cannot exist (P2).

* fix(harness): drop execution-record promise from report contract when receipts are disabled

The <report_contract> opening was emitted unconditionally, so a
verification.receipts_enabled=false subagent was told its report would
be cross-checked against an execution record that cannot exist in that
mode (terminal_receipts() returns None; no verdict, no ledger citation
line). The opening now follows receipts_enabled: enabled keeps the
cross-check language, disabled describes the handle-only review mode
(PR #5090 review, willem-bd P2).

* docs: record the prompt-layer trust-boundary self-check

Generalizes the PR #5090 review outcome: before adding prompt text, ask
of every data source in it what trust level it has and which channel it
should ride — model/user-influenceable values ride the untrusted
sanitized data channel, never framework-owned system text. Added to the
PR template (Agents/LangGraph surface) and agents/AGENTS.md.
2026-08-30 11:39:25 +08:00
Zeren Wang
4e35f0d1d4
feat(harness): deterministic tool receipts with model-visible ledger (RFC #4651, layer 1) (#4659)
* feat(harness): add deterministic tool receipts with model-visible ledger

Stamp an immutable per-call fact record (tool name, status, args/output
hashes, byte count, timestamp) onto every tool result via a new
ToolReceiptMiddleware, and inject the derived receipt ledger (r1..rN)
into the model context so subagent reports can cite executed actions.

- tool_receipt.py: receipt core (make/extract/render), newest-first
  budget eviction, ids derived from the append-only message stream
- ToolReceiptMiddleware: stamps ToolMessages directly or inside
  Command-wrapped results; hidden ledger injection mirrors
  DurableContextMiddleware; sits between ToolProgress and
  ToolErrorHandling with a build-time ordering guard
- config: new verification section (receipts on, judge off), config
  version 32 -> 33 with example/helm/docs updates

* feat(harness): split receipt rendering from stamping; address PR review

Review fixes (PR #4659):
- output_sha256 now uses sort_keys=True for structured content, matching
  the order-invariant args fingerprint
- stamping failures log at warning (silent ledger gaps would corrupt
  citations); tool execution remains never blocked
- _insert_after_leading_system_messages extracted to shared public
  message_utils.insert_after_leading_system_messages; both middlewares
  depend on it instead of a private cross-module helper
- code comments in English

RFC #4651 revision-2 alignment:
- receipts_render_mode config ('always' | 'delegation_only'): subagent
  chains always render the ledger (citations are produced there); the
  lead chain renders only while processing subagent results, removing
  the always-on token tax from ordinary turns
- receipts gain bounded args_preview/output_preview (<=200 chars, tail
  for output) so later typed claim bindings (tests_passed) can anchor
  to a specific recorded execution

* docs(harness): state receipt freshness caveat and vocabulary layering in module docstring

* merge: upstream/main — resolve AGENTS.md split, bump config_version to 34, drop unused receipt previews

- backend/AGENTS.md: take upstream's slimmed root guidance (#4799); move the
  ToolReceiptMiddleware chain entry into agents/middlewares/AGENTS.md and the
  verification.* hot-reload mention into config/AGENTS.md
- config.example.yaml + helm values/README: config_version 33 -> 34 so existing
  v33 configs get the outdated-config prompt (review: willem-bd)
- tool_receipt.py: drop args_preview/output_preview — no Layer 1 consumer reads
  them; re-add with the Layer 2 claim-binding consumer (review: willem-bd)

* docs(harness): cover receipt id renumbering after compaction in module docstring

Positional display ids are stable only while history is append-only;
compaction drops ToolMessages and the survivors renumber, so Layer 2
citation verification must resolve [rN] against the ledger as of the
citing turn (review: willem-bd, doc-only).

* chore(config): bump config_version to 35

main reached 34 via #4780 without the verification section; publishing
the new schema at the same number would silently skip the outdated-config
prompt for configs synced from main in that window (review: willem-bd).

* fix(skills): restore errno import dropped upstream in #4830

upstream/main adf6c422 uses errno.ENOTDIR in the drift guard but removed
the import, so the PR merge ref fails lint-backend (F821).

* fix(harness): harden tool receipts against forgery and turn-scope delegation_only

Address willem-bd's pre-merge review on #4659:

1. Untrusted receipt metadata: the gateway now strips the server-owned
   deerflow_tool_receipt key from external input messages; stamping always
   overwrites any tool-supplied value instead of preserving it; and
   extract_tool_receipts validates persisted receipt shapes (required typed
   fields, unknown keys ignored) so malformed entries are skipped instead of
   crashing render or passing as runtime-stamped evidence.

2. delegation_only no longer sticks on: _should_render now scopes the
   subagent_status scan to the current turn (messages after the latest
   genuine user message), so an old completed delegation stops rendering the
   ledger on later ordinary turns. The genuine-user predicate moves to
   message_utils.is_genuine_user_message, shared with input sanitization.

* fix(harness): stamp receipts outside short-circuiting tool middlewares

Address willem-bd's review on #4659: ToolReceiptMiddleware was registered
inside Guardrail/SandboxAudit/ReadBeforeWrite/ToolProgress, each of which
can return a ToolMessage without invoking its handler — blocked calls
(e.g. a read-before-write-denied write_file) never got a receipt, silently
gapping the ledger on a default-enabled path. SandboxAudit additionally
rebuilds medium-risk results, dropping an inner stamp.

ToolReceiptMiddleware is now the outermost wrap_tool_call layer in the
runtime tail. Normal results still carry deerflow_tool_meta (stamped by
ToolErrorHandling on the inner return path); short-circuit messages
self-stamp meta or fall back to message.status. The new invariant is
declared as ordering constraints in deerflow.extensions.ordering, with
composed-chain regression tests for a blocked write and a warn-rebuilt
bash result.
2026-08-23 15:43:37 +08:00
Aari
5ffc2d3e27
feat(mcp): complete durable task notifications and chat UI (#4833)
* feat(mcp): add reliable task notifications and cancellation

* feat(mcp): add background task chat UI

* fix(mcp): hide and sanitize task notification prompts

* fix(mcp): sanitize projected task names

* fix(mcp): harden task notifications and details

* fix(mcp): harden task lifecycle recovery

* fix(mcp): gate task UI and isolate cancellations

* test: scope plain-text response locator

* fix(mcp): align task notification boundaries

* fix(mcp): bound task delivery retries

* fix background task notification races
2026-08-22 16:53:32 +08:00
Baldwinzc
2e9ee329ce
fix(middleware): sanitize bare string blocks in list-form user content (#4668)
InputSanitizationMiddleware's text extraction only collected dict blocks
with type == "text", so a HumanMessage whose content list carried a bare
str item (a shape message_content_to_text treats as text and some IM/SDK
clients send) yielded no text at all — the request passed through
unwrapped and unescaped, letting forged framework tags (<system-reminder>
etc.) reach the model untouched.  The sibling rfind-fallback path in
_process_request already neutralized bare strings individually, and both
ToolResultSanitizationMiddleware and ToolOutputBudgetMiddleware treat
bare strings as text; the extraction helper was the odd one out.

Collect bare string blocks alongside text-block dicts (skipping empty
items, matching message_content_to_text), merging them into the single
sanitized text block on rebuild while interleaved non-text blocks keep
their positions.
2026-08-05 08:06:01 +08:00
Lee minjing
e225ad57d7
feat(uploads): lazy-load historical files via list_uploaded_files tool (#4174)
* feat(uploads): lazy-load historical files via list_uploaded_files tool

Replace per-turn injection of all historical upload metadata with on-demand
discovery via a new `list_uploaded_files` built-in tool, following the same
deferred-discovery pattern used by skills.

- Rename <uploaded_files> block to <current_uploads> (current-run files only)
- Add list_uploaded_files tool with include_outline: bool|list[str]
- Extract outline helpers to shared deerflow/utils/file_outline.py
- Update system prompt to reflect lazy-loading behaviour
- Historical file scan removed from UploadsMiddleware.before_agent()

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(uploads): clear uploaded_files state when no new files in current turn

When before_agent() returns None on empty turns, the LastValue
uploaded_files field retains the previous turn's filenames.
list_uploaded_files then incorrectly excludes those files as
"current-run" files, making them invisible until the next upload.

Fix: return {"uploaded_files": []} instead of None to explicitly
clear state. Add two-turn regression test covering the exact
scenario from review feedback.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: resolve CI lint errors and stale test assertion from merge

- Split long prompt line to fit 240-char limit
- Add missing `Any` import in list_uploaded_files_tool
- Remove unused `re` import in file_conversion (outline code moved)
- Remove unused `os` import in middleware test
- Fix test assertion: <uploaded_files> → <current_uploads> after main merge

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: resolve CI lint errors and stale test assertion from merge

- Split long prompt line to fit 240-char limit
- Add missing `Any` import in list_uploaded_files_tool
- Remove unused `re` import in file_conversion (outline code moved)
- Remove unused `os` import in middleware test
- Fix test assertion: <uploaded_files> → <current_uploads> after main merge

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: add current_uploads to input sanitization exempt tags

The lazy-loading PR renamed <uploaded_files> to <current_uploads>.
The anti-drift guard scans all framework XML blocks and requires each
to be either blocked or explicitly exempted. current_uploads wraps
trusted server-generated file metadata, not user input, so it belongs
in the exempt set.

Co-Authored-By: Claude <noreply@anthropic.com>

* test: regenerate replay golden after uploaded_files state change

before_agent now returns {"uploaded_files": []} instead of None,
adding uploaded_files to SSE values events. Regenerated via
DEERFLOW_WRITE_GOLDEN=1.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: review feedback — memory pipeline, stale tags, state clearing, nits

- Match both tags in memory stripping pipeline (uploaded_files|current_uploads)
- Remove stale uploaded_files from _BLOCKED_TAG_NAMES
- Clear uploaded_files on all before_agent early-return paths
- Fix ponytail: stray word in file_conversion re-export comment
- Remove dead total_omitted branch in _format_omitted_summary
- ruff format fixes

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: block current_uploads, sanitize only original user content

Per review feedback: instead of exempting <current_uploads> (which
allows user forgery), move it to _BLOCKED_TAG_NAMES and change
InputSanitizationMiddleware._process_request to scan only the
original user content (ORIGINAL_USER_CONTENT_KEY) when available.
Server-injected trusted blocks are no longer checked against the
blocked-tag denylist.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: clarify fallback reason in input sanitization comment

Co-Authored-By: Claude <noreply@anthropic.com>

* @
fix: third-round review feedback — state visibility, sanitization, regex, nits

- list_uploaded_files_tool: logger.warning instead of silent try/except
  on runtime.state read failure (High)
- input_sanitization_middleware: _extract_text_from_content skips empty
  text blocks to match message_content_to_text behaviour; rfind fallback
  path logs warning for observability (Medium)
- memory pipeline regexes: backreference (?P<tag>)(?P=tag) in
  message_processing.py and prompt.py (Low)
- file_conversion.py: re-export moved to top of file (Low)
- Tests: middleware→tool state bridge test; integrated forged-tag +
  multimodal sanitization tests

PR #4174 — Follow-up issues: #4212, #4213, #4214

Co-Authored-By: Claude <noreply@anthropic.com>
@

* @
fix: 4th-round review — denylist, sanitization, scandir, nits

- Add "uploaded_files" back to _BLOCKED_TAG_NAMES (old tag still processed by
  deermem; user forgery must be escaped) (consistency)
- Fix inaccurate rfind-fallback comment: UploadsMiddleware keeps string as
  string, fallback is unreachable for strings (doc fix)
- Distinguish "empty string key" (upload without text) from "non-string key"
  (caller forgery) so empty-text uploads never escape the server block (edge)
- Merge dual os.scandir(uploads_dir) calls into one list re-use (minor)
- Add comment on .md sibling skip known limitation: user-uploaded .md files
  whose stem collides with a converted doc are hidden (boundary, no code change)

Co-Authored-By: Claude <noreply@anthropic.com>
@

* @
fix: tighten rfind-failure fallback — distinguish server blocks from user blocks

When _extract_text_from_content and message_content_to_text disagree on
multimodal list content and rfind fails, use content[0] (server-injected
<current_uploads> block) vs content[1:] (user blocks) to sanitize only
user blocks.  Raw strings and non-standard dict blocks that
_extract_text_from_content misses are now also sanitized.

Non-distinguishable paths (< 2 text blocks, non-list content) still
degrade to full sanitization (safe — server block may be escaped but
user forgery never leaks).  All fallback paths log via logger.warning.

Decision 18 / willem-bd 4th-round comment #3

Co-Authored-By: Claude <noreply@anthropic.com>
@

* @
fix: correct comments referencing text_blocks → content in rfind fallback

Co-Authored-By: Claude <noreply@anthropic.com>
@

* fix: 5th-round review — dead code, subagent gating, integration test, perf, consistency

- Delete unreachable ORIGINAL_USER_CONTENT_KEY guard in rfind fallback
  branch (original_user_content guaranteed non-empty str at that point)
- Remove list_uploaded_files from BUILTIN_TOOLS; add include_upload_tool
  param to get_available_tools(), default True; task_tool.py passes False
  so subagents no longer receive a tool whose state exclusion is broken
- Add integration test exercising real create_agent graph (not mocked
  runtime.state) to verify LangGraph propagates before_agent state writes
  into ToolRuntime.state during same-turn tool calls
- Cache DirEntry.stat() st_size in candidates tuple to avoid second
  per-file syscall in the rendering loop
- Make the upload-tag pre-check case-insensitive (content_str.lower())
  to match _UPLOAD_BLOCK_RE re.IGNORECASE

PR #4174 — willem-bd 5th-round review items #1-#5

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(channels): pass files metadata through _human_input_message() for IM uploads

_human_input_message() was not passing additional_kwargs.files to the
downstream message. UploadsMiddleware read no files, wrote
uploaded_files=[], and list_uploaded_files reported same-run IM
attachments as historical files (fancyboi999 repro).

Fix: add files parameter to _human_input_message(), call site passes
files=uploaded. Regression test locks the contract.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(channels): remove legacy <uploaded_files> manual prepend to fix double-injection regression

Commit 8d86dbf6 added files= pass-through to UploadsMiddleware but
left the manual _format_uploaded_files_block() prepend in place.
Every IM attachment reached the model twice — once via the legacy
<uploaded_files> block and once via <current_uploads>.

This commit removes the manual prepend and the now-dead
_format_uploaded_files_block() function. UploadsMiddleware is the
sole upload-context producer for both IM and web paths.

Reported-by: fancyboi999 (PR review)
Co-Authored-By: Claude <noreply@anthropic.com>

* docs: update #4212 issue body to reflect completed fixes and narrowed remaining scope

* chore: remove temporary scratch file

* fix(middleware): neutralize user-derived values inside <current_uploads> block

Upload-derived filenames, paths, outline titles, and preview text are
interpolated verbatim inside the trusted <current_uploads> wrapper,
which InputSanitizationMiddleware exempts from sanitization. A crafted
filename or document heading containing blocked authority tags would
bypass the guardrail and enter model context as trusted framework data.

Fix: call neutralize_untrusted_tags() on all four user-derived values
inside _format_file_entry(), preserving the outer <current_uploads>
wrapper untouched.

Reported-by: fancyboi999 (P1 security review)
Co-Authored-By: Claude <noreply@anthropic.com>

* fix(middleware): neutralize extension labels in omitted-file summary

Files exceeding the 10-item context cap bypass _format_file_entry().
Their extensions, derived from user-controlled filenames via
_extension_label(), were interpolated verbatim into the trusted
<current_uploads> wrapper — another path for blocked authority tags
to escape the guardrail.

Fix: neutralize extension values inside _extension_label(), the
single extraction point for all extension labels.

Reported-by: fancyboi999 (P1 security review)
Co-Authored-By: Claude <noreply@anthropic.com>

* fix(tools): neutralize user-derived values in list_uploaded_files tool result

Apply neutralize_untrusted_tags() to every model-visible user-derived value
returned by list_uploaded_files: filename, virtual path, extension, outline
titles, outline preview lines, and omitted-file extension summary.

This closes the last remaining injection bypass in the upload lazy-loading
path - the <current_uploads> block and its omitted summary were already
neutralized (previous commits), but the list_uploaded_files tool produced
a second exit for the same attacker-controlled metadata that
ToolResultSanitizationMiddleware did not cover.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(tests): add missing include_upload_tool=False to task_tool mock assertions

PR #4174 added include_upload_tool parameter to get_available_tools().
task_tool.py correctly passes include_upload_tool=False for subagents
but 5 existing tests' assert_called_once_with expectations were not
updated, causing CI failures.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-22 14:02:56 +08:00
Aari
41b137c4c4
fix(security): block forged framework tags in the input guardrail (#4155)
* fix(security): block forged framework tags in the input guardrail

InputSanitizationMiddleware's _BLOCKED_TAG_NAMES neutralizes forged
framework tags in untrusted input, but missed soul, thinking_style, and
critical_reminders -- which the lead-agent system prompt's System-Context
Confidentiality section names as internal framework data -- and the
underscore spelling system_reminder emitted by the todo/terminal
middlewares (only the hyphen spelling was blocked). A user, or an
attacker-controlled web_fetch/web_search page via the shared
neutralize_untrusted_tags primitive, could forge these blocks. Add them.

* fix(security): cover framework authority blocks as a class, not a subset

The confidentiality section declares every framework structured tag trusted
("and all other structured tags"), so the denylist must cover the authority
blocks as a class. Add the live blocks still passing both sanitization paths
(clarification_system, self_update, response_style, citations, skill_index,
available_skills, disabled_skills, memory_tool_system, durable_context_data,
slash_skill_activation), and pin the set against drift with a test that scans
the framework source and fails when a new block is not blocked.

* fix(security): scan the whole harness for framework blocks, fail closed

The drift guard added in the previous revision scanned a hand-listed set of
source files. That is the same forgot-to-update-a-list root cause the guard was
meant to eliminate, one level up, and it failed exactly that way: tool_search.py
was not in the list, so <mcp_routing_hints> and <available-deferred-tools> —
both rendered into the lead-agent system prompt via the {deferred_tools_section}
/ {mcp_routing_hints_section} placeholders — passed both sanitization paths
unneutralized.

Replace the file list with a repo-wide scan plus an exemption set that states a
reason per tag. The point is the failure direction, not the breadth: a new
framework block anywhere in the harness now turns CI red until it is either
blocked or exempted on the record, where before a block emitted from an unlisted
file was silently unguarded.

The scan reads raw source rather than AST string literals on purpose: an
attributed block built as an f-string splits its '>' into a separate literal
chunk, so an AST-on-literals scan misses it (verified against
<consolidation_candidates>). Raw source has one comment false positive, exempted.

Exempted with reasons: leaf/wrapper elements; the memory-updater and summarizer
prompts, which are built from checkpointed state rather than the ModelRequest
this middleware rewrites, so blocking them here would be false coverage, not
protection; and the MindIE provider wire format, parsed out of model output.

The scan surfaced five further live authority blocks beyond the two reported.
Subagents reuse _build_runtime_middlewares and therefore share this denylist, so
their system-prompt blocks are in the same class: file_editing_workflow,
guidelines, output_format, working_directory. goal_continuation is a
framework-authored hidden HumanMessage injected into the lead agent.

Also loosen the scanner regex to match the tolerance of _BLOCKED_TAG_PATTERN so
an attributed block cannot hide from the guard.
2026-07-14 11:04:11 +08:00
AnoobFeng
446fa03801
fix(context): resolve context compress bug (#4065)
* fix(runtime): persist original human input outside model sanitization

* refactor(history): load thread messages by global event sequence

* fix(frontend): make summarization rescue a transient history bridge

* fix(frontend): old message not append tail

1. add identity anchor
2. add bridgeOrder

* fix(frontend): lint error fix

* fix: address review feedback and harden pagination coverage

- defer transient history ref writes until after render commit
- cover large middleware-only history scans
- verify infinite-query refetch recalculates page cursors
- document AI event types and anchor-weaving differences

* fix: harden message pagination and enrichment

- append unmatched live tails after canonical history
- warn and stop when pagination has_more lacks a cursor
- deep-copy restored UI messages to isolate model-facing content
- log invalid event sequence and non-advancing cursor errors
- pass user_id explicitly through event-store history queries
- cover middleware-only AI runs across memory, JSONL, and DB stores

* fix: address pagination review feedback

* fix(frontend): checkpoint has unknow redener content, optimize the anchor policy

* fix(frontend): unit test issue missed previously, remove the TanStack cache trimming

* fix(gateway): harden message history queries and provenance

- reject externally forged original_user_content metadata
- validate provenance metadata in upload and sanitization middleware
- make run lookups fail closed by default
- batch feedback queries by run ID
- align memory message filtering with persistent stores
2026-07-14 10:43:13 +08:00
AnoobFeng
47b0f604f4
feat(frontend):enhance the ask_clarification interaction with visualized card (#3956)
* feat(frontend): add structured human input cards for ask_clarification

Implement a reusable Human Input Card flow for ask_clarification while keeping
the existing text fallback for older clients and IM channels.

Backend:
- Add structured ToolMessage.artifact.human_input payloads for clarification requests.
- Preserve ToolMessage.content as the readable Markdown/text fallback.
- Normalize clarification options from native lists, JSON strings, plain strings,
  mixed scalar values, None, and missing options.
- Derive input_mode as choice_with_other when options exist, otherwise free_text.
- Keep disable_clarification non-interactive behavior as a plain ToolMessage with
  no human_input artifact.
- Cover artifact persistence and Gateway message metadata preservation in tests.

Frontend:
- Add human input protocol types, runtime guards, extractors, response builders,
  and thread-state helpers.
- Add reusable HumanInputCard with option buttons, free-text input, pending,
  read-only, disabled, and answered states.
- Render structured clarification cards from artifact.human_input, with Markdown
  fallback for malformed or legacy tool messages.
- Preserve line breaks in structured question/context/option text.
- Hide submitted clarification bridge messages from the chat UI via
  additional_kwargs.hide_from_ui.
- Send structured human_input_response metadata through the fourth sendMessage
  options argument, preserving run context in the third argument.
- Wire submissions for normal chats, custom agent chats, agent bootstrap chats,
  and sidecar chats.
- Derive answered state from raw thread.messages so hidden replies still update
  the original card.
- Clear pending state when the hidden reply arrives, dispatch is dropped, or a
  later async stream failure appears on thread.error.

* perf(frontend): optimize HumanInputCard UI interactions

- Support Enter key to submit text input (Shift+Enter for newline)
- Render question and context fields as Markdown instead of plain text
- Replace deprecated FormEventHandler type with structural typing

* test(frontend): add unit test cover optimize HumanInputCard UI interactions

* feat(frontend): disabled chatbox when has new human-input-card

* fix(style): lint error fix

* fix: sanitize hidden human input replies

- Preserve IME composition safety for human input card Enter submits
- Treat hidden human input responses as genuine user messages for sanitization
- Keep hidden card replies in memory filtering while excluding malformed/internal hidden messages
- Add regression coverage for card IME handling and hidden reply sanitization

* fix: tighten human input response validation

- Reject empty hidden human input response values
- Remove invalid list ARIA role from human input card options
- Add backend coverage for empty response payloads

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-06 22:34:41 +08:00
AochenShen99
442248dd06
feat: preserve durable context across summarization (#3887)
* feat: preserve durable context across summarization

* fix: harden durable context review gaps

* style: format delegation ledger live test

* chore: remove stale delegation ledger prefix

* fix: address durable context review feedback
2026-07-01 22:49:17 +08:00
Zhipeng Zheng
e7b88a97ed
fix(security): add input sanitization middleware for prompt-injection defense (#3630) (#3662)
InputSanitizationMiddleware: escapes blocked XML tags in user messages (<system> -> &lt;system&gt;) — de-identify, don't reject (AWS PII-style). Wraps user input in plain-text boundary markers (--- BEGIN/END USER INPUT ---) per OWASP structured-prompt guidance. System-Context Confidentiality clause prevents LLM from revealing internal instructions. Boundary-marker layer hardened against delimiter injection (self-suppression + break-out). Multimodal content blocks preserved during sanitization.

Addresses review feedback from @fancyboi999 and @WillemJiang:

1. Boundary-marker injection: strict startswith+endswith idempotency, neutralize user-supplied BEGIN/END tokens, forged-idempotency bypass fixed

2. Multimodal data loss: _rebuild_content preserves interleaved non-text blocks

3. CI fixes: replay_provider strips boundary markers before hashing, middleware chain assertions updated

4. Lint: ruff format applied on Linux (slice spacing + f-string collapse)

Test: 71 input_sanitization + 1 replay_golden + 14 tool_error + 93 tool_output_budget = 179/179 passed
2026-06-21 19:01:43 +08:00