5 Commits

Author	SHA1	Message	Date
Wenchao An	4c05e88c52	feat: add inline @ references with multiple skill activation (#6063) ... * feat(frontend): unify composer references with an @ picker * feat: edit references inline and activate multiple skills * docs: describe inline references and multiple skill selection * fix: prevent sent drafts from returning and repair CI checks * fix: reconcile inline references and address review feedback * fix: preserve references through discovery errors and polish	2026-09-30 23:09:05 +08:00
hataa	1cd3eb742b	feat(authz): enforce skill authorization at assembly and slash-activation (#4063 Phase 3) (#4541) ... * feat(authz): enforce skill authorization at assembly and slash-activation (#4063 Phase 3) Phase 3 / Skills — the second of three resource-type PRs (Models, Skills, Sandbox). The RBAC provider already maps "skill" → config key "skills" (rbac.py _RESOURCE_POLICY_KEYS), so no schema change is needed. Layer 1 (assembly-time, mirrors Phase 1B): - filter_available_skills_by_authorization() in skill_filter.py filters the skill-name allowlist (set[str] | None) by the provider's "skill" policy. In lead agent, runs after _available_skill_names() so the catalog (describe_skill) and SkillActivationMiddleware share one filtered set. In subagent executor _load_skills(), filters config.skills before disk load. - filter_resources_by_authorization() in enforcement.py — generic version of filter_tools_by_authorization for any resource with a name attribute. Layer 2 (slash-activation): - No new middleware code needed. SkillActivationMiddleware._resolve_activation already checks "reference.name not in self._available_skills". Since Layer 1 filters the allowlist, denied skills cannot be slash-activated. authorization.enabled: false is a complete no-op. available_skills=None (no agent allowlist) + enabled: resolves all skill names from config and filters. 12 new tests + 243 existing authz/skills tests pass. * test(subagents): wait on the chained future instead of polling done() in caller-loop teardown test * feat(authz): enforce action-scoped skill:activate at runtime and reuse the catalog loader for filter candidates * feat(authz): arm skill:activate on the subagent chain, unify the client skill-surface user, and record the Layer 2 reversal in the design notes * chore(authz): align the subagent filter's fallback user with the chain's bucket convention * feat(authz): gate autonomous skill loading on skill:activate, close the bootstrap assembly leak, and reuse one provider per subagent assembly Review round 4 (willem-bd, CHANGES_REQUESTED): - [P1] Autonomous load path: describe_skill filters its match set on the action-scoped skill:activate decision (all-denied is indistinguishable from "no match"), and ToolErrorHandlingMiddleware stamps skill_context entries only when the action allows it — a denied SKILL.md read gets a skill_context_denied marker instead, so durable context, allowed-tools policy, and autonomous secret bindings never activate a denied skill. File visibility itself stays Layer 1 (sandbox-projection semantics). - [P1] Bootstrap assembly: build_middlewares/apply_prompt_template now receive the authorization-filtered set instead of the raw _BOOTSTRAP_SKILL_NAMES, and an empty skill_names index is preserved when deferred discovery is on rather than round-tripping through None into the legacy full-metadata prompt that re-advertises a denied skill (deferred-off keeps the legacy rendering; the filtered empty allowlist blanks it on denial). - [P2] SubagentExecutor resolves ResolvedSkillAuthorization once per assembly (_resolve_skill_authorization, memoized per executor) and shares it across the Layer 1 filter (authorization=), the activation middleware, and both new gates — uncached provider factories can no longer hand the layers different instances/policy snapshots. All gates share one helper (skill_filter.skill_activation_allowed) so slash, describe, and skill-file-load cannot drift; provider errors keep the configured fail-closed/fail-open policy. skill_authorization is threaded through _build_runtime_middlewares into both runtime builders. Tests: describe gating (deny/fail-closed/fail-open/disabled), read stamping (denied marker vs entry) + extract_skills skip without warning, bootstrap three-way param (allow/deny x deferred on/off), lead + subagent chain wiring for the stamp gate, and the distinct-instance provider factory repro. Each fix is mutation-verified (reverting it fails its test). Full A/B suite run: no new failures vs HEAD. * feat(authz): re-authorize persisted skill_context before reuse and route async paths through aauthorize Review round 5 (willem-bd): - [P1] Persisted entries: skill_context survives across runs, so both consumers now re-check the action-scoped skill:activate decision before reuse. SkillToolPolicyMiddleware._active_skills_for_paths skips a denied skill (same skip as a disabled/unallowlisted one; all-denied keeps the existing fail-closed-to-builtins semantics) and _in_context_secret_sources binds no secrets for a denied entry. skill_authorization is threaded into SkillToolPolicyMiddleware on both chains. - [P2] Async provider API: skill_activation_allowed_async mirrors the sync helper through await provider.aauthorize() (same pattern as authorize_sandbox_execution_async). Async hooks precompute an activation_decisions map on the event loop (slash target + persisted entry names, one batch of awaits) and hand it to the worker-thread handler, which consults the map instead of the sync API: SkillActivationMiddleware.awrap_model_call (activation + secret binding), ToolErrorHandlingMiddleware.awrap_tool_call (_amaybe_stamp awaits the decision before stamping), and SkillToolPolicyMiddleware's awrap_model_call/awrap_tool_call (entry names + slash source name). describe_skill becomes a StructuredTool with both a sync function (invoke -> authorize) and a coroutine (ainvoke -> aauthorize), so async agent execution never routes a loop-affine provider through the sync API while sync callers keep their semantics. Tests: allow-read-then-deny-next-run regressions for secret binding and allowed-tools (partial deny does not poison, all-denied fails closed to builtins), and an _AsyncOnlyProvider (sync authorize always fails, aauthorize works) driving four async-path assertions (activation, stamp allow/deny, describe, tool policy). All six fixes are mutation-verified; full A/B suite run against HEAD shows no new failures. * fix(authz): authorize skill activation by declared Skill.name, not directory basename ShenAC-SAC's P2: bundled skills may declare a name that differs from their directory (skills/public/vercel-deploy-claimable declares name: vercel-deploy), but the runtime gates derived authorization targets from the container path. - New deerflow/skills/container_registry.py: build_container_path_registry() + canonical_skill_name() — the one path-to-Skill.name resolution shared by the activation, tool-policy, and read-stamping middlewares. - ToolErrorHandlingMiddleware resolves the read path through the (user-scoped, via new user_id kwarg threaded from both runtime builders) registry before the skill:activate decision, sync and async (registry lookup in a worker thread, aauthorize() on the loop). RBAC allow by the declared name now activates; allow by the directory name does not. - SkillToolPolicyMiddleware async hooks canonicalize the policy paths first (three-phase: thread registry -> loop aauthorize -> thread filter reusing the registry; the cached per-model-step decision still short-circuits), so the decision map is keyed by the names _active_skills_for_paths checks — no more map miss falling back to the sync provider API from the thread. _entry_names removed: persisted entry["name"] is path-derived by construction; entries are resolved by path instead. - SkillActivationMiddleware collects slash names (already canonical) plus persisted entry *paths*, canonicalized off-loop via _canonical_names_for_paths(). Regressions (test_skills_authorization.py 45->50): sync stamp target, RBAC allow declared-name/deny directory-name both directions, async stamp via _AsyncOnlyProvider, composed async slash -> tool-policy keeps the skill's tools with zero sync fallback, persisted-entry secret binding survives the name mismatch. All three mutations (stamp target, policy map key, entry canonicalization) fail their tests. * fix(authz): close the sync-fallback class on skill activation paths willem-bd's two P2s on the canonical-name fix, plus a fourth instance of the same class found while sweeping the membership: - SkillToolPolicyMiddleware: a transient prepass registry failure is now preserved (_REGISTRY_LOAD_FAILED marker) — the worker-side filter applies its own fail-closed treatment instead of silently retrying storage, which recovered skills absent from the decision map and fell back to the synchronous provider API from the thread (fail-open retaining a denied skill's allowed-tools). - SkillActivationMiddleware: an empty entry-path list no longer pays a full skill-tree scan on ordinary authorization-enabled async model steps (previously a passive path with zero registry I/O). - 4th instance (reproduced with the PR fixtures, then fixed): the entry prepass failure on the secret-binding path let the consumer-side _load_skill_registry_by_path silently recover and resolve entry names outside the decision map — the same sync fallback, fail-open binding the aauthorize-denied skill's declared secret. The fix goes one step further than failure propagation: the prepass hands down the registry *snapshot* the decision map was keyed by, and entry sources resolve against that snapshot, so a map miss is impossible by construction (this also closes the narrower window where both loads succeed but disagree because storage changed mid-step). The slash source is deliberately unaffected: it does not consult the decision map (validated once at activation) and keeps its fresh per-call load. - Class membership swept: skill_activation_allowed() sync call sites are exactly four (stamping sync path and describe's sync implementation are legitimately sync); the only two async-reachable fallback points (the _activation_allowed helpers) are both construction-guarded now. Structural hardening (typed async-batch decision object where a map miss is fail-closed by construction) recorded in the plan notes as a follow-up, anchored at those two helpers. - _RegistryArg type alias replaces the degenerate dict|object|None union in the policy middleware signatures. Tests (test_skills_authorization.py 50->54): transient prepass failure -> builtins-only with exactly one storage attempt and zero provider calls; ordinary async step -> zero registry scans; two-call scenario -> entry binding zeroed while the slash binding survives; entries resolve against the prepass snapshot under a mid-step rename. Mutations verified red: marker reverted to None, over-broad slash kill, snapshot bypass. * fix(authz): correct secret-binding reference points and gate the rendered skill reminder Review round 9 (willem-bd) plus construction-audit follow-ups on the same class — every async divergence between two loads of the same data now has an explicit, pinned reference point: - Slash secret source resolves from a FRESH post-activation registry: the prepass snapshot is taken before the aauthorize awaits, so binding the slash source from it would activate NEW_KEY content while injecting OLD_KEY when a skill's declared secrets change in that window. The 'one scan per step' bonus that motivated sharing the snapshot is withdrawn — consistency wins. The two secret sources resolve independently: a transient fresh-load failure zeroes only the slash binding; entries keep binding from their own snapshot. - Same-skill dominance: when a skill is both the slash source (fresh) and a persisted entry (snapshot), only the activation-era declaration binds — unioning both eras injected a secret the skill no longer declares. Aligns with the tool-policy rule (slash activation dominates for the run); different-skill entries still stack additively. - The rendered 'Active skills' reminder (DurableContextMiddleware) no longer advertises skills the provider denies — surface consistency with describe_skill. Publication/consumption split: the activation middleware publishes per-step path-keyed skill:activate decisions into the run context (async: reused from the prepass snapshot, zero extra I/O; sync: one scan + authorize() only when entries exist — passive steps stay scan-free), the renderer consumes them and filters its render copy (state untouched; unpublished = permissive, matching the other run-context carriers). New carrier key listed in REDACTED_CONTEXT_KEYS with a redaction regression. - Wiring regressions on both chains for everything added above (user_id to the stamp gate, skill_authorization + token pairing + activation- before-durable ordering to the durable renderer): each mutation verified red (M4-M13 across this change set). - Tests 54->65: slash post-activation lookup, source independence, dominance, sync-chain sync-API path, render filter (async/sync/ disabled with load-count assertions), chain wiring x2, redaction listing. Plan notes record rounds 9-2..6 including the reference-point post-mortem and the audit axes exhausted. * fix(authz): anchor same-skill dominance on the slash identity, not binding results Review round 10 (willem-bd) plus a self-audit extension the new checklist items caught in the first fix itself: - R10 (P2): the same-skill exclusion was derived from successfully BOUND slash sources. When the prepass snapshots required-secrets [OLD_KEY] and the operator strips the declaration during the await window, the slash source binds nothing (required_secrets gate) — the exclusion set stayed empty and the stale entry view kept injecting OLD_KEY for a skill that now declares no secrets. The exclusion now derives from the AUTHENTICATED slash activation identity (run-context source path, owner-token checked): the identity's declared name resolves from the fresh registry first, then the entry snapshot, and joins the exclusion set regardless of binding success. - Self-audit (value->gate->downstream on the fix itself): the declared name can also change mid-run (same path, foo->bar). A name-anchored exclusion then holds only the new name while the snapshot entry resolves the same path to the old one — both eras bound again. The PATH is the stable identity shared by the activation and every earlier read: entries at the slash identity's path are now excluded before registry resolution (exclude_paths), independent of what either version calls the skill; the name-based set remains for cross-path same-name shadowing. Tests 65->67: no-secrets-era dominance (ACTIVE secrets None under the OLD_KEY-to-empty interleaving), path-anchored dominance across a mid-run rename (only the activation-era key binds). Mutations verified red: identity-derived exclusion disabled (M14), path exclusion disabled (M16). * test(authz): pass explicit config to lead middleware wiring test --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-09-30 19:45:07 +08:00
georgelichen	24001e80b7	fix(skills): safely tokenize portable allowed-tools patterns (#4984) ... * fix(skills): accept portable frontmatter forms * fix(skills): normalize portable tool names * Safely preserve parenthesized portable skill tool patterns Portable Agent Skills declarations such as Bash(tvly *) contain spaces inside a command pattern. Keep those patterns as single literal entries while preserving exact names from the existing YAML-list form, so skill loading no longer fragments valid metadata or rewrites mixed-case MCP tools. Constraint: DeerFlow's current skill policy matches exact tool names and does not inspect Bash arguments Constraint: Agent Skills scalar syntax uses whitespace-separated entries with parenthesized command patterns Rejected: raw.split() | fragments Bash(tvly *) into unrelated tool names Rejected: normalize YAML-list entries | breaks case-sensitive MCP/runtime tool names Rejected: map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Bash(...) entries literal and inactive until DeerFlow has an explicit command-pattern authorization model Tested: 175 focused parser, validation, installer, review, loader, and tool-policy tests; Ruff check and format; compileall; git diff --check Not-tested: Full backend suite stopped at pre-existing Windows mode assertion test_runtime_config_store_file_is_owner_only Related: #4912 * Preserve exact custom tool names in portable skill parsing Portable scalar frontmatter needs alias normalization for known DeerFlow-compatible names, but generic case conversion corrupts MCP and custom tool identifiers. The tokenizer also treated quoted or escaped parentheses as structural delimiters, rejecting valid command patterns. Preserve unknown names and parse quoted or escaped patterns without broadening Bash(...) into bash. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Parenthesized patterns remain literal because argument-level authorization is not implemented Rejected: Generic CamelCase-to-snake_case for every scalar | rewrites custom/MCP names Rejected: Map Bash(...) to bash | broadens command-scoped declarations into unrestricted shell access Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add an explicit alias before supporting another portable tool name; keep command-pattern authorization separate Tested: 225 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests Related: #4984; #4912 * Preserve case-sensitive exact tool authorities Case-folding a scalar declaration before alias lookup can turn literal write into write_file, substituting a different runtime authority. Keep exact portable spellings as aliases and preserve lowercase, custom, and MCP names; strengthen activation coverage for spaced Bash patterns and command fragments. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Bash(...) remains literal and inactive because command-pattern authorization is not implemented Rejected: Case-insensitive alias lookup | maps lowercase runtime tools onto built-in authorities Rejected: Broaden the parser into command-pattern authorization | outside this PR's scope Confidence: high Scope-risk: narrow Reversibility: clean Directive: Add aliases only for documented portable spellings; preserve all other scalar names verbatim Tested: 226 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite remains affected by unrelated Windows permissions/path and missing Lark CLI tests; GitNexus index refresh remains stale Related: #4984; #5016297602 * Support portable Glob and Grep skill aliases Portable Agent Skills commonly declare Glob and Grep, but DeerFlow exposes the runtime tools as glob and grep. Add explicit exact-spelling aliases and activation coverage so imported skills retain search-tool access without broad normalization. Constraint: Runtime skill policy uses exact tool-name matching Constraint: Alias conversion is limited to documented portable spellings Rejected: Case-fold all scalar names | can substitute custom or MCP authorities Rejected: Map arbitrary names by convention | breaks exact runtime compatibility Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the alias table explicit and preserve unknown scalar names verbatim Tested: 228 skills tests passed, 1 skipped; Ruff check; Ruff format --check; compileall; git diff --check Not-tested: Full backend suite has unrelated environment failures on Windows; GitNexus index reports stale line mappings Related: #4984; #5026257899 --------- Co-authored-by: kriptoburak <kriptoburak@users.noreply.github.com>	2026-08-28 08:58:59 +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
Huixin615	65afc9b1d2	fix(skills): apply allowed-tools only to active skills (#4098) ... * fix(skills): scope allowed-tools to active skills * fix(skills): tolerate stale active skill paths * chore: retrigger CI * fix(skills): document policy activation limits * perf(skills): reuse per-step tool policy decisions * fix(skills): harden runtime tool policy contracts * fix(skills): redact cached policy decisions * fix(skills): make slash tool policy authoritative * fix(skills): preserve policy-safe discovery tools * test(skills): cover explicit task delegation policy	2026-07-16 14:12:02 +08:00