From 1cd3eb742be1ce99985977f631e642663fe12899 Mon Sep 17 00:00:00 2001 From: hataa <79907651+hata33@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:45:07 +0800 Subject: [PATCH] feat(authz): enforce skill authorization at assembly and slash-activation (#4063 Phase 3) (#4541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../deerflow/agents/lead_agent/agent.py | 90 +- .../middlewares/durable_context_middleware.py | 30 +- .../skill_activation_middleware.py | 343 +- .../agents/middlewares/skill_context.py | 12 + .../skill_tool_policy_middleware.py | 198 +- .../tool_error_handling_middleware.py | 171 +- .../harness/deerflow/authz/__init__.py | 5 +- .../harness/deerflow/authz/enforcement.py | 40 + .../harness/deerflow/authz/skill_filter.py | 237 ++ backend/packages/harness/deerflow/client.py | 53 +- .../deerflow/runtime/secret_context.py | 39 + .../deerflow/skills/container_registry.py | 40 + .../harness/deerflow/skills/describe.py | 101 +- .../harness/deerflow/subagents/executor.py | 75 + .../tests/test_authorization_enforcement.py | 83 + backend/tests/test_client.py | 1 + backend/tests/test_models_authorization.py | 11 +- .../test_skill_tool_policy_middleware.py | 2 +- backend/tests/test_skills_authorization.py | 3082 +++++++++++++++++ ...able-authorization-implementation-notes.md | 456 ++- 20 files changed, 4983 insertions(+), 86 deletions(-) create mode 100644 backend/packages/harness/deerflow/authz/skill_filter.py create mode 100644 backend/packages/harness/deerflow/skills/container_registry.py create mode 100644 backend/tests/test_skills_authorization.py diff --git a/backend/packages/harness/deerflow/agents/lead_agent/agent.py b/backend/packages/harness/deerflow/agents/lead_agent/agent.py index 99dcf386bd..9a227fab7a 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/agent.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/agent.py @@ -492,6 +492,7 @@ def build_middlewares( mcp_routing_middleware: AgentMiddleware | None = None, user_id: str | None = None, authorization_provider=None, + skill_authorization=None, extensions=None, subagent_execution_capacity: int | None = None, ): @@ -521,6 +522,11 @@ def build_middlewares( to ``SkillActivationMiddleware`` so it can resolve per-user custom skills. authorization_provider: Provider already resolved for assembly-time filtering. Reused by the execution-time authorization middleware. + skill_authorization: Provider + principal already resolved for the + skill visibility filter. Passed through to + ``SkillActivationMiddleware`` so explicit slash activation also + enforces the action-scoped ``skill:activate`` decision (custom + providers may distinguish visibility from activation). subagent_execution_capacity: Startup-frozen process capacity used to keep advertised and enforced task concurrency aligned after reloads. extensions: Loaded extensions whose middleware contributions are merged @@ -543,6 +549,15 @@ def build_middlewares( runtime_middleware_kwargs["owns_agent_skill_projection"] = False if authorization_provider is not None: runtime_middleware_kwargs["authorization_provider"] = authorization_provider + if skill_authorization is not None: + # Shared with the runtime chain (ToolErrorHandlingMiddleware gates the + # skill-file-load stamp on the same skill:activate decision the + # activation middleware enforces for slash commands). + runtime_middleware_kwargs["skill_authorization"] = skill_authorization + if user_id is not None: + # ToolErrorHandlingMiddleware canonicalizes skill-read paths through + # the same user-scoped registry the activation/policy middlewares use. + runtime_middleware_kwargs["user_id"] = user_id if authorization_provider is not None and deferred_setup is not None: runtime_middleware_kwargs["deferred_setup"] = deferred_setup middlewares = build_lead_runtime_middlewares(**runtime_middleware_kwargs) @@ -571,6 +586,7 @@ def build_middlewares( app_config=resolved_app_config, user_id=user_id, slash_source_owner_token=slash_source_owner_token, + skill_authorization=skill_authorization, ) ) @@ -592,6 +608,10 @@ def build_middlewares( app_config=resolved_app_config, user_id=user_id, slash_source_owner_token=slash_source_owner_token, + # Persisted skill_context entries are re-authorized against the + # skill:activate decision before their allowed-tools apply (the + # policy may have changed since the entry was stamped). + skill_authorization=skill_authorization, ) ) @@ -607,6 +627,11 @@ def build_middlewares( inject_tool_artifacts=resolved_app_config.tool_artifacts.enabled and resolved_app_config.tool_artifacts.inject_model_context, task_continuity_enabled=getattr(getattr(resolved_app_config, "task_continuity", None), "enabled", False) is True, pii_redaction_config=getattr(resolved_app_config, "pii_redaction", None), + # The rendered "Active skills" reminder hides entries the provider + # denies; decisions are published per step by the activation + # middleware under the shared chain token. + skill_authorization=skill_authorization, + entry_decisions_owner_token=slash_source_owner_token, ) ) @@ -972,6 +997,46 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le if isinstance(config.get("context"), dict): config["context"]["subagent_enabled"] = subagent_enabled available_skills = _available_skill_names(agent_config, is_bootstrap) + + # Phase 3: enforce skill authorization (Layer 1). Filter the skill + # allowlist by the provider's "skill" policy so denied skills never + # appear in , can never be describe_skill'd, and cannot be + # slash-activated (SkillActivationMiddleware checks this set). When + # authorization is disabled, this is a no-op. ``available_skills=None`` + # means "no agent-level allowlist" (all enabled skills); authorization + # still constrains via filter_resources in that case. The resolved + # provider/principal are threaded into ``SkillActivationMiddleware`` so + # explicit activation additionally enforces the action-scoped + # ``skill:activate`` decision (visibility != activation for custom + # providers). + from deerflow.authz.skill_filter import filter_available_skills_by_authorization, resolve_skill_authorization + + skill_authorization = resolve_skill_authorization(cfg, resolved_app_config) + # Resolve the filter's candidate names through the cached catalog loader + # instead of letting the filter rescan storage: the enabled-skills load + # below (``_load_enabled_available_skills``) uses the same + # config-identity cache, so a build pays one skill-tree walk when the + # cache is cold and none when warm — the filter's own uncached fallback + # double-pays it on every build (notably costly on NFS/CSI roots). + candidate_skill_names = None + if available_skills is None and skill_authorization is not None: + try: + from deerflow.agents.lead_agent.prompt import get_enabled_skills_for_config + + candidate_skill_names = [s.name for s in get_enabled_skills_for_config(resolved_app_config, user_id=resolved_user_id)] + except Exception: + logger.warning("Failed to pre-load enabled skills for authorization candidates", exc_info=True) + # Leave None: the filter resolves candidates itself and applies + # its fail-closed semantics when that resolution also fails. + + available_skills = filter_available_skills_by_authorization( + available_skills, + context=cfg, + app_config=resolved_app_config, + user_id=resolved_user_id, + candidate_skill_names=candidate_skill_names, + authorization=skill_authorization, + ) # Custom agent model from agent config (if any), or None to let _resolve_model_name pick the default agent_model_name = agent_config.model if agent_config and agent_config.model else None @@ -1080,6 +1145,7 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le bootstrap_skills, enabled=skill_search_enabled, container_base_path=container_base_path, + skill_authorization=skill_authorization, ) chat_model = create_chat_model(name=model_name, thinking_enabled=thinking_enabled, app_config=resolved_app_config, attach_tracing=False) raw_tools = get_available_tools(model_name=model_name, subagent_enabled=subagent_enabled, app_config=resolved_app_config, chat_model=chat_model) + [setup_agent] @@ -1111,7 +1177,12 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le config, model_name=model_name, agent_name=agent_name, - available_skills=set(_BOOTSTRAP_SKILL_NAMES), + # The authorization-filtered bootstrap set, not the raw + # _BOOTSTRAP_SKILL_NAMES: when policy denies "bootstrap" this is an + # empty set, which the activation middleware's membership gate + # treats as "no skills activatable" (a raw {"bootstrap"} would + # re-admit the denied skill to slash activation). + available_skills=available_skills, memory_enabled=memory_enabled, owns_agent_skill_projection=False, app_config=resolved_app_config, @@ -1119,17 +1190,28 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le mcp_routing_middleware=mcp_routing_middleware, user_id=resolved_user_id, authorization_provider=_authz_provider, + skill_authorization=skill_authorization, subagent_execution_capacity=subagent_execution_capacity, ) system_prompt = apply_prompt_template( subagent_enabled=subagent_enabled, max_concurrent_subagents=max_concurrent_subagents, max_total_subagents=max_total_subagents, - available_skills=set(_BOOTSTRAP_SKILL_NAMES), + # Same filtered set as build_middlewares above; the legacy prompt + # path also consults it, and an unfiltered {"bootstrap"} would let + # the full-metadata rendering advertise a policy-denied skill. + available_skills=available_skills, app_config=resolved_app_config, deferred_names=setup.deferred_names, user_id=resolved_user_id, - skill_names=skill_setup.skill_names or None, + # Preserve the (possibly empty) index when deferred discovery is + # on: converting an empty set back to None would fall through to + # the legacy full-metadata prompt, which reloads every enabled + # skill from storage and re-advertises the denied one. When + # deferred discovery is off, None keeps the legacy rendering and + # the filtered available_skills above already blanks it for a + # denial. + skill_names=skill_setup.skill_names if skill_search_enabled else None, allowed_subagents=allowed_subagents, subagent_execution_capacity=subagent_execution_capacity, interaction_policy=interaction_policy, @@ -1187,6 +1269,7 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le enabled_skills, enabled=skill_search_enabled, container_base_path=container_base_path, + skill_authorization=skill_authorization, ) # # Withhold ``update_agent`` from runs triggered by webhook channels @@ -1251,6 +1334,7 @@ def _assemble_lead_agent(config: RunnableConfig, *, app_config: AppConfig) -> Le mcp_routing_middleware=mcp_routing_middleware, user_id=resolved_user_id, authorization_provider=_authz_provider, + skill_authorization=skill_authorization, subagent_execution_capacity=subagent_execution_capacity, ) system_prompt = apply_prompt_template( diff --git a/backend/packages/harness/deerflow/agents/middlewares/durable_context_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/durable_context_middleware.py index 5f3756c6b4..2723c481cc 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/durable_context_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/durable_context_middleware.py @@ -308,6 +308,8 @@ class DurableContextMiddleware(AgentMiddleware[AgentState]): inject_tool_artifacts: bool = True, task_continuity_enabled: bool = False, pii_redaction_config: PiiRedactionConfig | None = None, + skill_authorization=None, + entry_decisions_owner_token: str | None = None, ) -> None: super().__init__() self._task_continuity_enabled = task_continuity_enabled @@ -315,6 +317,8 @@ class DurableContextMiddleware(AgentMiddleware[AgentState]): self._skills_root = _normalize_skills_root(skills_container_path) self._skill_read_tool_names = frozenset(DEFAULT_SKILL_FILE_READ_TOOL_NAMES if skill_file_read_tool_names is None else skill_file_read_tool_names) self._inject_tool_artifacts = inject_tool_artifacts + self._skill_authorization = skill_authorization + self._entry_decisions_owner_token = entry_decisions_owner_token def release_policy_parameters(self) -> dict[str, object]: """Describe the normalized inputs that govern capture and injection.""" @@ -324,8 +328,32 @@ class DurableContextMiddleware(AgentMiddleware[AgentState]): "task_continuity_enabled": self._task_continuity_enabled, "inject_tool_artifacts": self._inject_tool_artifacts, "pii_redaction_enabled": bool(self._pii_redaction_config and self._pii_redaction_config.enabled), + "skill_entry_render_filter": bool(self._skill_authorization), } + def _renderable_skills(self, request: ModelRequest, entries: list) -> list: + """Filter persisted ``skill_context`` entries the provider now denies. + + The rendered "Active skills" reminder must not advertise a skill whose + ``skill:activate`` decision is denied — the same denial + ``describe_skill`` applies to the catalog. Decisions come from the + skill-activation middleware's per-step publication (path-keyed, owner + token authenticated); this middleware performs no provider or storage + work of its own. Nothing published (authorization disabled, no + entries, or a foreign carrier) renders unchanged — absent is + permissive, matching the other run-context carriers. A published + ``False`` hides the entry from the reminder only; state is untouched. + """ + if not entries or self._skill_authorization is None or not self._entry_decisions_owner_token: + return entries + from deerflow.runtime.secret_context import read_skill_entry_decisions + + context = getattr(getattr(request, "runtime", None), "context", None) + decisions = read_skill_entry_decisions(context, owner_token=self._entry_decisions_owner_token) + if decisions is None: + return entries + return [entry for entry in entries if isinstance(entry, dict) and decisions.get(posixpath.normpath(entry.get("path") or ""), False)] + @override def before_model(self, state: AgentState, runtime: Runtime) -> dict | None: return self._capture(state, runtime) @@ -385,7 +413,7 @@ class DurableContextMiddleware(AgentMiddleware[AgentState]): data_block = _render_durable_context_data( redact_text(state.get("summary_text"), self._pii_redaction_config), state.get("delegations") or [], - state.get("skill_context") or [], + self._renderable_skills(request, state.get("skill_context") or []), (state.get("task_notes") or {}) if self._task_continuity_enabled else None, state.get("task_history") if self._task_continuity_enabled else None, artifacts=artifacts, diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py index fa848ac7e3..0f8b97f37b 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/skill_activation_middleware.py @@ -38,6 +38,7 @@ from deerflow.skills.types import SKILL_MD_FILE, SecretRequirement, Skill, Skill from deerflow.utils.messages import get_original_user_content_text, is_real_user_message if TYPE_CHECKING: + from deerflow.authz.skill_filter import ResolvedSkillAuthorization from deerflow.config.app_config import AppConfig logger = logging.getLogger(__name__) @@ -45,6 +46,15 @@ logger = logging.getLogger(__name__) _SLASH_SKILL_ACTIVATION_KEY = "slash_skill_activation" _SLASH_SKILL_ACTIVATION_TARGET_ID_KEY = "slash_skill_activation_target_id" +# Async-prepass registry load already attempted and failed. Handed down so +# the secret-binding resolution does not silently recover with a fresh load: +# entry names resolved from a recovered registry would miss the (empty) +# decision map and fall back to the synchronous provider API from the worker +# thread — for a loop-affine provider a denial becomes a fail-open allow. +# Entries bind nothing for that call instead (fail closed by construction). +# Same marker pattern as SkillToolPolicyMiddleware._REGISTRY_LOAD_FAILED. +_REGISTRY_LOAD_FAILED = object() + # _SECRETS_BINDING_AUDIT_KEY: last audited binding (skill and secret names only, # never values) so unchanged bindings are not re-recorded each call. # The shared slash-source context contract holds the latest slash activation, @@ -100,6 +110,7 @@ class SkillActivationMiddleware(AgentMiddleware): app_config: AppConfig | None = None, user_id: str | None = None, slash_source_owner_token: str, + skill_authorization: ResolvedSkillAuthorization | None = None, ) -> None: super().__init__() if not isinstance(slash_source_owner_token, str) or not slash_source_owner_token: @@ -108,6 +119,117 @@ class SkillActivationMiddleware(AgentMiddleware): self._app_config = app_config self._user_id = user_id self._slash_source_owner_token = slash_source_owner_token + self._skill_authorization = skill_authorization + + def _activation_allowed(self, skill_name: str, *, activation_decisions: dict[str, bool] | None = None) -> bool: + """Action-scoped ``skill:activate`` check for explicit slash activation. + + ``_available_skills`` is the Layer 1 *visibility* set (filter_resources, + action-agnostic); an action-aware custom provider may expose a skill + there while denying its activation. Mirrors ``_authorize_model_name``'s + second ``authorize("model", "use")`` check. Provider errors follow the + configured fail-closed / fail-open policy. ``None`` (authorization + disabled) allows — membership in the visibility set already decided. + Delegates to the shared ``skill_activation_allowed`` so the slash path, + ``describe_skill``, and the skill-file-load path cannot drift. + + *activation_decisions* carries decisions precomputed on the event loop + via ``aauthorize()`` by ``awrap_model_call``: the threaded handler then + consults them instead of calling the synchronous ``authorize()`` (wrong + API for loop-affine providers). Names absent from the map fall back to + the synchronous check, which is the correct API for the sync + ``wrap_model_call`` path. + """ + if activation_decisions is not None and skill_name in activation_decisions: + return activation_decisions[skill_name] + from deerflow.authz.skill_filter import skill_activation_allowed + + return skill_activation_allowed(self._skill_authorization, skill_name) + + async def _collect_activation_decisions(self, names) -> dict[str, bool] | None: + """Precompute ``skill:activate`` decisions on the event loop. + + Async middleware paths offload the blocking handler (skill-tree reads) + to a worker thread, but the provider decision belongs on the loop with + ``aauthorize()``. Collects every candidate name the handler may need — + the slash target and the persisted ``skill_context`` entries — so one + batch of awaits covers both the activation and secret-binding gates. + ``None`` means authorization is disabled (nothing to precompute). + """ + if self._skill_authorization is None: + return None + candidates = {name for name in names if isinstance(name, str) and name} + if not candidates: + return None + from deerflow.authz.skill_filter import skill_activation_allowed_async + + decisions: dict[str, bool] = {} + for name in sorted(candidates): + decisions[name] = await skill_activation_allowed_async(self._skill_authorization, name) + return decisions + + def _candidate_activation_targets(self, request: ModelRequest) -> tuple[list[str], list[str]]: + """Split the activation candidates into (names, entry_paths). + + Loop-safe (no storage I/O): the slash reference is parsed straight + from the latest real user message — its name is already canonical, + because ``_resolve_activation`` matches it against the registry's + ``Skill.name`` set. The persisted ``skill_context`` entries contribute + their *paths*: the stamped ``entry["name"]`` is path-derived (the + directory name) and can differ from the declared ``Skill.name``, so + the consumer (``_in_context_secret_sources``) resolves the path + through the live registry — the decision map must be keyed by the + same canonical names, resolved off-loop by the caller. + """ + names: list[str] = [] + entry_paths: list[str] = [] + messages = list(request.messages) + for index in range(len(messages) - 1, -1, -1): + if _is_user_activation_target(messages[index]): + content = get_original_user_content_text(messages[index].content, messages[index].additional_kwargs) + reference = parse_slash_skill_reference(content) + if reference is not None: + names.append(reference.name) + break + state = getattr(request, "state", None) or {} + try: + entries = state.get("skill_context") or [] + except AttributeError: + entries = [] + for entry in entries: + if not isinstance(entry, dict): + continue + path = entry.get("path") + if isinstance(path, str) and path: + entry_paths.append(path) + return names, entry_paths + + def _canonical_names_for_paths(self, paths: list[str]) -> tuple[list[str], dict[str, Skill] | object]: + """Canonical ``Skill.name`` for the entry paths plus the registry they + came from (thread-only; disk I/O). + + Returns ``(names, registry)`` where *registry* is the snapshot the + names were resolved from — the async caller hands it to the secret + binding resolution so entries are resolved against the SAME snapshot + the decision map was keyed by (a map miss is then impossible by + construction, closing the sync-API fallback divergence). On storage + failure returns ``([], _REGISTRY_LOAD_FAILED)``: entries must bind + nothing rather than be re-resolved from a silently recovered fresh + load. Returns immediately for an empty list: this helper runs on + every authorization-enabled async model step, and an ordinary request + (no slash reference, no persisted entries, no supplied secrets) must + not pay a full skill-tree scan per LLM call. + """ + if not paths: + return [], None + try: + from deerflow.skills.container_registry import build_container_path_registry, canonical_skill_name + + registry = build_container_path_registry(self._storage()) + except Exception: + logger.exception("Failed to load skills while collecting activation candidates") + return [], _REGISTRY_LOAD_FAILED + return [name for name in (canonical_skill_name(registry, path) for path in paths) if name is not None], registry def release_policy_parameters(self) -> dict[str, object]: return { @@ -145,7 +267,7 @@ class SkillActivationMiddleware(AgentMiddleware): raise FileNotFoundError(resolved_file) return resolved_file.read_text(encoding="utf-8") - def _resolve_activation(self, text: str) -> _ActivationResolution | None: + def _resolve_activation(self, text: str, *, activation_decisions: dict[str, bool] | None = None) -> _ActivationResolution | None: reference = parse_slash_skill_reference(text) if reference is None: return None @@ -159,6 +281,11 @@ class SkillActivationMiddleware(AgentMiddleware): return _ActivationResolution(failure_message=f"Skill `/{reference.name}` is installed but disabled. Enable it before using slash activation.") if self._available_skills is not None and reference.name not in self._available_skills: return _ActivationResolution(failure_message=f"Skill `/{reference.name}` is not available for this agent.") + if not self._activation_allowed(reference.name, activation_decisions=activation_decisions): + # Visible in the Layer 1 set but denied by the action-scoped + # skill:activate policy — same user-facing message as the + # membership denial so the reason is not leaked. + return _ActivationResolution(failure_message=f"Skill `/{reference.name}` is not available for this agent.") resolved = resolve_slash_skill( text, @@ -268,7 +395,7 @@ Follow this skill before choosing a general workflow. Load supporting resources """ return isinstance(run_context, dict) and run_context.get(_SLASH_SKILL_ACTIVATION_RUN_KEY) == run_key - def _find_activation_target(self, messages: list, *, run_context: dict | None = None) -> tuple[int, HumanMessage, _ActivationResolution, str] | None: + def _find_activation_target(self, messages: list, *, run_context: dict | None = None, activation_decisions: dict[str, bool] | None = None) -> tuple[int, HumanMessage, _ActivationResolution, str] | None: if not messages: return None @@ -293,7 +420,7 @@ Follow this skill before choosing a general workflow. Load supporting resources return None content = get_original_user_content_text(target.content, target.additional_kwargs) - resolution = self._resolve_activation(content) + resolution = self._resolve_activation(content, activation_decisions=activation_decisions) if resolution is None: return None return target_index, target, resolution, run_key @@ -321,9 +448,9 @@ Follow this skill before choosing a general workflow. Load supporting resources except Exception: logger.warning("Failed to record slash skill activation audit event", exc_info=True) - def _prepare_model_request(self, request: ModelRequest, *, hook: str) -> tuple[ModelRequest | AIMessage | None, _Activation | None]: + def _prepare_model_request(self, request: ModelRequest, *, hook: str, activation_decisions: dict[str, bool] | None = None) -> tuple[ModelRequest | AIMessage | None, _Activation | None]: run_context = self._run_context(request) - target_and_resolution = self._find_activation_target(list(request.messages), run_context=run_context) + target_and_resolution = self._find_activation_target(list(request.messages), run_context=run_context, activation_decisions=activation_decisions) if target_and_resolution is None: return None, None @@ -359,12 +486,19 @@ Follow this skill before choosing a general workflow. Load supporting resources messages.insert(target_index, activation_msg) return request.override(messages=messages), activation - def _handle_model_request(self, request: ModelRequest, *, hook: str) -> tuple[ModelRequest | AIMessage, _Activation | None]: - prepared, activation = self._prepare_model_request(request, hook=hook) + def _handle_model_request( + self, + request: ModelRequest, + *, + hook: str, + activation_decisions: dict[str, bool] | None = None, + entry_registry: dict[str, Skill] | object | None = None, + ) -> tuple[ModelRequest | AIMessage, _Activation | None]: + prepared, activation = self._prepare_model_request(request, hook=hook, activation_decisions=activation_decisions) if isinstance(prepared, AIMessage): return prepared, None effective = prepared if prepared is not None else request - self._resolve_secret_bindings(effective, activation, hook=hook) + self._resolve_secret_bindings(effective, activation, hook=hook, activation_decisions=activation_decisions, entry_registry=entry_registry) if activation is not None: record_skill_usage(getattr(request, "runtime", None), self._usage_snapshot(activation)) return effective, activation @@ -395,7 +529,15 @@ Follow this skill before choosing a general workflow. Load supporting resources break return response - def _resolve_secret_bindings(self, request: ModelRequest, activation: _Activation | None, *, hook: str) -> None: + def _resolve_secret_bindings( + self, + request: ModelRequest, + activation: _Activation | None, + *, + hook: str, + activation_decisions: dict[str, bool] | None = None, + entry_registry: dict[str, Skill] | object | None = None, + ) -> None: """Recompute the per-run secret injection set (binding point A+, #3861/#3914). Sources, unioned on every model call: @@ -440,15 +582,65 @@ Follow this skill before choosing a general workflow. Load supporting resources request_secrets = extract_request_secrets(context) sources: list[tuple[str, tuple[SecretRequirement, ...]]] = [] if request_secrets: - registry = self._load_skill_registry_by_path() - if registry is not None: + # Two secret sources, two deliberately DIFFERENT registry + # reference points: + # + # Slash source — always a fresh, post-activation registry. Its + # binding must match the skill content the activation just read; + # the prepass snapshot is taken BEFORE the aauthorize awaits, so + # resolving the slash source from it would activate NEW_KEY + # content while binding OLD_KEY when a skill's declared secrets + # change in that window (review: keep the fresh slash lookup + # separate from the entry snapshot). The slash source never + # consults the decision map, so snapshot consistency buys nothing + # here. + slash_registry = self._load_skill_registry_by_path() + # Entry sources — the snapshot the decision map was keyed by + # (a map miss is impossible by construction), or an empty + # registry after a failed prepass (bind nothing rather than + # silently recover with a fresh load whose names would miss the + # map and fall back to the sync provider API). ``None`` (sync + # chain / prepass not run) falls back to the same fresh registry + # the slash source uses — on the sync chain the synchronous + # authorize() is the correct API, so no divergence exists there. + if entry_registry is _REGISTRY_LOAD_FAILED: + entry_registry_effective: dict[str, Skill] = {} + elif entry_registry is not None: + entry_registry_effective = entry_registry + else: + entry_registry_effective = slash_registry if slash_registry is not None else {} + # The two sources resolve independently — a transient failure of + # the fresh slash lookup must not zero the entry sources (they + # have their own snapshot and never consult that registry). + slash_path = read_slash_skill_source_path(context, owner_token=self._slash_source_owner_token) + if slash_registry is not None: # Slash source: exempt from the ``secrets-autonomous`` opt-out # (explicit ceremony), but still enabled + allowlist checked. - slash_path = read_slash_skill_source_path(context, owner_token=self._slash_source_owner_token) - slash_skill = self._resolve_registry_skill(registry, slash_path, require_autonomous=False) + slash_skill = self._resolve_registry_skill(slash_registry, slash_path, require_autonomous=False) if slash_skill is not None: sources.append((slash_skill.name, tuple(slash_skill.required_secrets))) - sources.extend(self._in_context_secret_sources(request, registry)) + # Entry sources resolve independently. The same-skill exclusion is + # anchored on the AUTHENTICATED slash activation identity — not on + # binding success, and not on the declared name alone: a + # slash-activated skill whose current version declares no secrets + # binds nothing itself, yet must still suppress its stale entry + # view, and a mid-run rename (same path, new declared name) would + # otherwise slip a name-anchored exclusion. The PATH is the stable + # identity shared by the activation and every earlier read, so the + # identity path always excludes; the resolved names (fresh registry + # first, then the entry snapshot) additionally cover the cross-path + # same-name shadowing case. + slash_bound = {name for name, _ in sources} + identity_paths: frozenset[str] = frozenset() + if isinstance(slash_path, str) and slash_path: + normalized_identity = posixpath.normpath(slash_path) + identity_paths = frozenset({normalized_identity}) + for registry in (slash_registry, entry_registry_effective): + identity = registry.get(normalized_identity) if isinstance(registry, dict) else None + if identity is not None: + slash_bound.add(identity.name) + break + sources.extend(self._in_context_secret_sources(request, entry_registry_effective, activation_decisions=activation_decisions, exclude_names=frozenset(slash_bound), exclude_paths=identity_paths)) injected: dict[str, str] = {} bound_skills: set[str] = set() @@ -505,13 +697,12 @@ Follow this skill before choosing a general workflow. Load supporting resources call rather than trusting stale caller-supplied data. """ try: - storage = self._storage() - skills = storage.load_skills(enabled_only=False) - container_root = storage.get_container_root() + from deerflow.skills.container_registry import build_container_path_registry + + return build_container_path_registry(self._storage()) except Exception: logger.exception("Failed to load skills while resolving secret bindings") return None - return {posixpath.normpath(skill.get_container_file_path(container_root)): skill for skill in skills} def _resolve_registry_skill(self, registry: dict[str, Skill], path: object, *, require_autonomous: bool) -> Skill | None: """Resolve a container path to a live registry skill eligible for secret @@ -541,13 +732,35 @@ Follow this skill before choosing a general workflow. Load supporting resources return None return skill - def _in_context_secret_sources(self, request: ModelRequest, registry: dict[str, Skill]) -> list[tuple[str, tuple[SecretRequirement, ...]]]: + def _in_context_secret_sources( + self, + request: ModelRequest, + registry: dict[str, Skill], + *, + activation_decisions: dict[str, bool] | None = None, + exclude_names: frozenset[str] = frozenset(), + exclude_paths: frozenset[str] = frozenset(), + ) -> list[tuple[str, tuple[SecretRequirement, ...]]]: """Map ``ThreadState.skill_context`` entries to declared-secret sources. Entries are references to skills the model actually loaded in this thread. Each is re-validated against the live registry so a skill that was disabled, uninstalled, opted out, or removed from the agent's - allowlist after being read stops binding immediately. + allowlist after being read stops binding immediately. The + ``skill:activate`` decision is re-checked here as well: ``skill_context`` + persists across runs, and an entry stamped under an earlier policy must + not keep binding secrets after the provider starts denying activation. + + *exclude_names* carries the slash-activated skill names: explicit slash + activation dominates for the rest of the run (the same rule the + tool-policy middleware applies), and the slash source is resolved from + a fresher registry than the entry snapshot — letting the same skill + contribute both sources would bind two eras of its declarations when + they change across the prepass→activation window. *exclude_paths* + carries the authenticated slash identity's container path — the stable + identity across mid-run renames, where the same path's old and new + versions carry different declared names and a name-anchored exclusion + alone would slip. """ state = getattr(request, "state", None) or {} try: @@ -560,9 +773,21 @@ Follow this skill before choosing a general workflow. Load supporting resources for entry in entries: if not isinstance(entry, dict): continue - skill = self._resolve_registry_skill(registry, entry.get("path"), require_autonomous=True) + entry_path = entry.get("path") + normalized_entry_path = posixpath.normpath(entry_path) if isinstance(entry_path, str) and entry_path else None + if normalized_entry_path is not None and normalized_entry_path in exclude_paths: + # The slash-activated identity owns this path; its entry view + # must not widen or stale it under any declared name. + continue + skill = self._resolve_registry_skill(registry, entry_path, require_autonomous=True) if skill is None or skill.name in seen: continue + if skill.name in exclude_names: + # The slash source already binds this skill's activation-era + # declaration; its entry view must not widen or stale it. + continue + if not self._activation_allowed(skill.name, activation_decisions=activation_decisions): + continue seen.add(skill.name) sources.append((skill.name, tuple(skill.required_secrets))) return sources @@ -605,18 +830,92 @@ Follow this skill before choosing a general workflow. Load supporting resources request: ModelRequest, handler: Callable[[ModelRequest], ModelResponse], ) -> ModelResponse | AIMessage: + # Sync-chain twin of the async publication: when authorization is + # enabled and persisted entries exist, resolve one registry snapshot + # plus the synchronous authorize() decisions and publish the path-keyed + # map for the durable renderer. Passive steps (no entries) publish + # nothing and pay no I/O, mirroring the async empty-paths guard. + if self._skill_authorization is not None: + _, entry_paths = self._candidate_activation_targets(request) + if entry_paths: + names, registry = self._canonical_names_for_paths(entry_paths) + decisions = self._collect_sync_activation_decisions(names) + self._publish_entry_decisions(request, entry_paths, registry, decisions) prepared, activation = self._handle_model_request(request, hook="wrap_model_call") if isinstance(prepared, AIMessage): return prepared return self._stamp_usage(handler(prepared), activation) + def _publish_entry_decisions( + self, + request: ModelRequest, + entry_paths: list[str], + entry_registry: dict[str, Skill] | object | None, + activation_decisions: dict[str, bool] | None, + ) -> None: + """Publish per-step ``skill:activate`` decisions keyed by entry path. + + The durable-context renderer consumes this map so the model-visible + "Active skills" reminder stops advertising skills the provider now + denies. Keyed by normalized container path (the renderer's entry + identity); a prepass registry failure, an unresolvable path, or a + missing decision all publish ``False`` — hide rather than advertise. + """ + run_context = self._run_context(request) + if run_context is None: + return + from deerflow.runtime.secret_context import write_skill_entry_decisions + + decisions: dict[str, bool] = {} + registry = entry_registry if isinstance(entry_registry, dict) else {} + collected = activation_decisions or {} + for path in entry_paths: + normalized = posixpath.normpath(path) + skill = registry.get(normalized) + decisions[normalized] = collected.get(skill.name, False) if skill is not None else False + write_skill_entry_decisions(run_context, decisions, owner_token=self._slash_source_owner_token) + + def _collect_sync_activation_decisions(self, names: list[str]) -> dict[str, bool] | None: + """Synchronous authorize() decisions for *names* (sync chain only).""" + if self._skill_authorization is None or not names: + return None + from deerflow.authz.skill_filter import skill_activation_allowed + + return {name: skill_activation_allowed(self._skill_authorization, name) for name in names} + @override async def awrap_model_call( self, request: ModelRequest, handler: Callable[[ModelRequest], Awaitable[ModelResponse]], ) -> ModelResponse | AIMessage: - prepared, activation = await asyncio.to_thread(self._handle_model_request, request, hook="awrap_model_call") + # Resolve the skill:activate decisions on the event loop with the + # provider's async API, then hand them to the worker thread: the + # blocking handler (skill-tree reads) keeps its to_thread offload while + # loop-affine providers still receive aauthorize() — a synchronous + # authorize() from the thread is the wrong API for them. Entry paths + # are canonicalized through the registry in a worker thread first so + # the decision map is keyed by declared Skill.name (the same names the + # threaded handler's registry lookups produce), not directory names. + # The snapshot those names came from travels with the decisions: the + # secret-binding resolution reuses it for the entry sources, so a map + # miss is impossible by construction (and a failed load means entries + # bind nothing — no silent fresh-load recovery). + entry_registry: dict[str, Skill] | object | None = None + if self._skill_authorization is not None: + slash_names, entry_paths = self._candidate_activation_targets(request) + canonical_names, entry_registry = await asyncio.to_thread(self._canonical_names_for_paths, entry_paths) + activation_decisions = await self._collect_activation_decisions([*slash_names, *canonical_names]) + self._publish_entry_decisions(request, entry_paths, entry_registry, activation_decisions) + else: + activation_decisions = None + prepared, activation = await asyncio.to_thread( + self._handle_model_request, + request, + hook="awrap_model_call", + activation_decisions=activation_decisions, + entry_registry=entry_registry, + ) if isinstance(prepared, AIMessage): return prepared return self._stamp_usage(await handler(prepared), activation) diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_context.py b/backend/packages/harness/deerflow/agents/middlewares/skill_context.py index 414e547cf2..0c1998b3e8 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/skill_context.py +++ b/backend/packages/harness/deerflow/agents/middlewares/skill_context.py @@ -20,6 +20,13 @@ _SKILL_FILE_NAME = "SKILL.md" # here the match fails on the leading marker and the captured description comes back empty. _FRONT_MATTER_RE = re.compile(r"^\ufeff?---\s*\n(.*?)\n---\s*\n", re.DOTALL) SKILL_CONTEXT_ENTRY_KEY = "skill_context_entry" +# Stamped instead of SKILL_CONTEXT_ENTRY_KEY when the read SKILL.md belongs to +# a skill whose action-scoped ``skill:activate`` decision is denied: the file +# content was served (Layer 1 visibility allows it) but the skill must not +# activate — no durable skill_context entry, tool policy, or secret binding. +# extract_skills uses it to skip the entry without the "missing skill read +# metadata" warning that an unstamped read would otherwise produce. +SKILL_CONTEXT_DENIED_KEY = "skill_context_denied" logger = logging.getLogger(__name__) @@ -160,6 +167,11 @@ def extract_skills( expected_path = skill_paths_by_id.get(tool_call_id) if expected_path is None: continue + if (message.additional_kwargs or {}).get(SKILL_CONTEXT_DENIED_KEY): + # The skill:activate decision denied this read's skill — the + # middleware stamped the denial instead of entry metadata on + # purpose, so this is not the "missing metadata" bug shape. + continue metadata = read_skill_entry_metadata(message.additional_kwargs) if metadata is None: content = message.content if isinstance(message.content, str) else "" diff --git a/backend/packages/harness/deerflow/agents/middlewares/skill_tool_policy_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/skill_tool_policy_middleware.py index c5d27280b3..3d9c0f65f1 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/skill_tool_policy_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/skill_tool_policy_middleware.py @@ -34,6 +34,18 @@ _POLICY_SOURCE_SLASH = "slash" _POLICY_SOURCE_SKILL_CONTEXT = "skill_context" _POLICY_SOURCES = frozenset({_POLICY_SOURCE_PASSIVE, _POLICY_SOURCE_SLASH, _POLICY_SOURCE_SKILL_CONTEXT}) _MISSING_POLICY_DECISION = object() +# Async-prepass registry load already attempted and failed: the async hook must +# not let the worker-side filter silently retry storage — a successful retry +# would resolve skills whose names are absent from the (empty) decision map and +# fall back to the synchronous provider API from the thread, which for a +# loop-affine provider turns a denial into a fail-open allow. Tri-state marker +# in the style of _MISSING_POLICY_DECISION. +# Registry argument handed down the policy-resolution call chain: a loaded +# snapshot (dict), the load-failure marker, or None ("load it here" — the +# sync-path behavior). +type _RegistryArg = dict[str, Skill] | object | None + +_REGISTRY_LOAD_FAILED = object() _TOOL_SEARCH_NAME = "tool_search" type _PolicySignature = tuple[str, tuple[str, ...]] @@ -56,6 +68,7 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): app_config: AppConfig | None = None, user_id: str | None = None, slash_source_owner_token: str, + skill_authorization=None, ) -> None: super().__init__() if not isinstance(slash_source_owner_token, str) or not slash_source_owner_token: @@ -64,8 +77,79 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): self._app_config = app_config self._user_id = user_id self._slash_source_owner_token = slash_source_owner_token + self._skill_authorization = skill_authorization self._decision_owner_token = secrets.token_urlsafe(24) + def _activation_allowed(self, skill_name: str, *, activation_decisions: dict[str, bool] | None = None) -> bool: + """Action-scoped ``skill:activate`` decision for one skill name. + + Persisted ``skill_context`` entries were authorized when they were + stamped, but the policy may have changed since — every entry is + re-authorized before its allowed-tools declaration is applied. + *activation_decisions* carries decisions precomputed on the event loop + via ``aauthorize()`` (async hooks); names absent from the map fall back + to the synchronous check, which is the correct API for the sync hooks. + """ + if self._skill_authorization is None: + return True + if activation_decisions is not None and skill_name in activation_decisions: + return activation_decisions[skill_name] + from deerflow.authz.skill_filter import skill_activation_allowed + + return skill_activation_allowed(self._skill_authorization, skill_name) + + async def _collect_activation_decisions(self, names) -> dict[str, bool] | None: + """Precompute ``skill:activate`` decisions on the event loop (async hooks). + + The policy resolution itself runs in a worker thread (storage reads), + but the provider decision belongs on the loop with ``aauthorize()``. + *names* must already be canonical (``Skill.name``) — the caller + resolves the policy paths through the live registry off-loop first, + because the decision consumers (``_active_skills_for_paths``) check + the registry skill's declared name. Authorizing a path-derived name + here would miss the map and fall back to the synchronous provider + call from the worker thread (wrong API for loop-affine providers). + """ + if self._skill_authorization is None: + return None + candidates = {name for name in names if isinstance(name, str) and name} + if not candidates: + return None + from deerflow.authz.skill_filter import skill_activation_allowed_async + + decisions: dict[str, bool] = {} + for name in sorted(candidates): + decisions[name] = await skill_activation_allowed_async(self._skill_authorization, name) + return decisions + + def _resolve_policy_registry(self, paths: tuple[str, ...]) -> tuple[dict[str, Skill] | object, list[str]]: + """Load the path-keyed registry and canonicalize the policy paths. + + Blocking (skill-tree read): worker thread only. Returns the registry, + the canonical ``Skill.name`` for every resolvable path, or — when the + load fails — the ``_REGISTRY_LOAD_FAILED`` marker with no names. The + marker preserves the failure: the async caller hands it down so + ``_active_skills_for_paths`` applies its fail-closed treatment instead + of retrying storage (a transient-failure retry that succeeds would + resolve skills missing from the decision map and fall back to the + synchronous provider API). Unresolvable paths yield no name: + ``_active_skills_for_paths`` skips them before the activation check, + so no decision is needed for them. + """ + try: + from deerflow.skills.container_registry import build_container_path_registry + + registry = build_container_path_registry(self._storage()) + except Exception: + logger.exception("Failed to load active skills for allowed-tools policy") + return _REGISTRY_LOAD_FAILED, [] + canonical: list[str] = [] + for path in paths: + skill = registry.get(posixpath.normpath(path)) + if skill is not None: + canonical.append(skill.name) + return registry, canonical + def _storage(self) -> SkillStorage: if self._user_id is not None: return get_or_new_user_skill_storage(self._user_id, app_config=self._app_config) @@ -73,13 +157,9 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): return get_or_new_skill_storage(app_config=self._app_config) return get_or_new_skill_storage() - def _active_policy(self, request: ModelRequest | ToolCallRequest) -> _PolicySignature: - context = getattr(getattr(request, "runtime", None), "context", None) - slash_path = read_slash_skill_source_path(context, owner_token=self._slash_source_owner_token) - if slash_path is not None: - return _POLICY_SOURCE_SLASH, (slash_path,) - - paths: list[str] = [] + @staticmethod + def _skill_context_entries(request: ModelRequest | ToolCallRequest) -> list: + """Persisted ``skill_context`` entries from agent state (possibly empty).""" state = getattr(request, "state", None) if state is None: state = {} @@ -93,28 +173,50 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): if not isinstance(entries, (list, tuple)): logger.warning("Invalid skill_context shape for skill tool policy: %s", type(entries).__name__) entries = [] - for entry in entries: + return list(entries) + + def _active_policy(self, request: ModelRequest | ToolCallRequest) -> _PolicySignature: + context = getattr(getattr(request, "runtime", None), "context", None) + slash_path = read_slash_skill_source_path(context, owner_token=self._slash_source_owner_token) + if slash_path is not None: + return _POLICY_SOURCE_SLASH, (slash_path,) + + paths: list[str] = [] + for entry in self._skill_context_entries(request): if isinstance(entry, dict) and isinstance(entry.get("path"), str): paths.append(entry["path"]) if paths: return _POLICY_SOURCE_SKILL_CONTEXT, tuple(paths) return _POLICY_SOURCE_PASSIVE, () - def _active_skills_for_paths(self, paths: tuple[str, ...]) -> tuple[list[Skill], bool]: + def _active_skills_for_paths( + self, + paths: tuple[str, ...], + *, + activation_decisions: dict[str, bool] | None = None, + registry: _RegistryArg = None, + ) -> tuple[list[Skill], bool]: if not paths: return [], False - try: - storage = self._storage() - skills = storage.load_skills(enabled_only=False) - container_root = storage.get_container_root() - except Exception: - logger.exception("Failed to load active skills for allowed-tools policy") - # A real active reference exists but cannot be authorized. Signal a - # policy failure so callers retain only framework-safe tools. + if registry is _REGISTRY_LOAD_FAILED: + # The async prepass already attempted the load and failed. Do not + # retry here: a successful retry would resolve skills absent from + # the prepass-built decision map and silently fall back to the + # synchronous provider API from this worker thread. Preserve the + # failure with the same treatment as a first-load failure below. return [], True + if registry is None: + try: + from deerflow.skills.container_registry import build_container_path_registry + + registry = build_container_path_registry(self._storage()) + except Exception: + logger.exception("Failed to load active skills for allowed-tools policy") + # A real active reference exists but cannot be authorized. Signal a + # policy failure so callers retain only framework-safe tools. + return [], True - registry = {posixpath.normpath(skill.get_container_file_path(container_root)): skill for skill in skills} active: list[Skill] = [] seen: set[str] = set() for path in paths: @@ -128,6 +230,13 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): if self._available_skills is not None and skill.name not in self._available_skills: logger.warning("Active skill is outside the agent allowlist for allowed-tools policy: %s", path) continue + # Phase 3: skill_context persists across runs — an entry stamped + # under an earlier policy must not keep applying its allowed-tools + # declaration after the provider starts denying activation (same + # skip-and-continue treatment as a disabled or unallowlisted skill). + if not self._activation_allowed(skill.name, activation_decisions=activation_decisions): + logger.info("Active skill is denied by the skill:activate policy for allowed-tools policy: %s", path) + continue if skill.name in seen: continue seen.add(skill.name) @@ -137,8 +246,14 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): return [], True return active, False - def _allowed_names_for_paths(self, paths: tuple[str, ...]) -> set[str] | None: - active_skills, policy_failed = self._active_skills_for_paths(paths) + def _allowed_names_for_paths( + self, + paths: tuple[str, ...], + *, + activation_decisions: dict[str, bool] | None = None, + registry: _RegistryArg = None, + ) -> set[str] | None: + active_skills, policy_failed = self._active_skills_for_paths(paths, activation_decisions=activation_decisions, registry=registry) if policy_failed: return set(ALWAYS_AVAILABLE_BUILTIN_TOOL_NAMES) allowed = allowed_tool_names_for_skills(active_skills) @@ -192,6 +307,8 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): request: ModelRequest | ToolCallRequest, *, policy: _PolicySignature | None = None, + activation_decisions: dict[str, bool] | None = None, + registry: _RegistryArg = None, ) -> set[str] | None: resolved_policy = self._active_policy(request) if policy is None else policy _, paths = resolved_policy @@ -199,7 +316,7 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): decision = self._read_policy_decision(context, resolved_policy) if decision is not _MISSING_POLICY_DECISION: return decision - return self._allowed_names_for_paths(paths) + return self._allowed_names_for_paths(paths, activation_decisions=activation_decisions, registry=registry) def _filter_model_request( self, @@ -207,10 +324,15 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): *, policy: _PolicySignature | None = None, refresh_decision: bool = False, + activation_decisions: dict[str, bool] | None = None, + registry: _RegistryArg = None, ) -> ModelRequest: resolved_policy = self._active_policy(request) if policy is None else policy _, paths = resolved_policy - allowed = self._allowed_names_for_paths(paths) if refresh_decision else self._allowed_names(request, policy=resolved_policy) + if refresh_decision: + allowed = self._allowed_names_for_paths(paths, activation_decisions=activation_decisions, registry=registry) + else: + allowed = self._allowed_names(request, policy=resolved_policy, activation_decisions=activation_decisions, registry=registry) if refresh_decision: self._store_policy_decision(request, resolved_policy, allowed) if allowed is None: @@ -325,11 +447,23 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): if not paths: self._store_policy_decision(request, policy, None) return await handler(request) + # Three phases, each on the right executor: the registry lookup + # (skill-tree read) and the policy filter run in a worker thread, + # while the skill:activate re-authorization decisions are awaited on + # the event loop (aauthorize) for the *canonical* skill names the + # registry resolved — keying the map by path-derived names would miss + # and fall back to the sync provider API from the thread (wrong API + # for loop-affine providers). The loaded registry is reused by the + # filter so the whole hook costs one skill-tree scan. + registry, canonical_names = await asyncio.to_thread(self._resolve_policy_registry, paths) + activation_decisions = await self._collect_activation_decisions(canonical_names) filtered = await asyncio.to_thread( self._filter_model_request, request, policy=policy, refresh_decision=True, + activation_decisions=activation_decisions, + registry=registry, ) return await handler(filtered) @@ -357,7 +491,25 @@ class SkillToolPolicyMiddleware(AgentMiddleware[AgentState]): policy = self._active_policy(request) if not policy[1]: return await handler(request) - allowed = await asyncio.to_thread(self._allowed_names, request, policy=policy) + # Reuse the current model step's cached decision when present — only a + # cache miss pays the three-phase resolution below (mirrors the sync + # wrap_tool_call, which reads the same cache inside _allowed_names). + cached = self._read_policy_decision(self._runtime_context(request), policy) + if cached is not _MISSING_POLICY_DECISION: + allowed = cached + else: + # Same three-phase shape as awrap_model_call: canonical names + # resolved off-loop, decisions awaited on the loop, blocking + # policy resolution in the thread with the registry reused. + registry, canonical_names = await asyncio.to_thread(self._resolve_policy_registry, policy[1]) + activation_decisions = await self._collect_activation_decisions(canonical_names) + allowed = await asyncio.to_thread( + self._allowed_names, + request, + policy=policy, + activation_decisions=activation_decisions, + registry=registry, + ) blocked = self._blocked_tool_message(request, allowed=allowed) if blocked is not None: return blocked diff --git a/backend/packages/harness/deerflow/agents/middlewares/tool_error_handling_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/tool_error_handling_middleware.py index 7d2d3d5378..2ec75bac85 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/tool_error_handling_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/tool_error_handling_middleware.py @@ -1,5 +1,6 @@ """Tool error handling middleware and shared runtime middleware builders.""" +import asyncio import logging import secrets from collections.abc import Awaitable, Callable @@ -13,7 +14,9 @@ from langgraph.prebuilt.tool_node import ToolCallRequest from langgraph.types import Command from deerflow.agents.middlewares.skill_context import ( + SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY, + _skill_name_from_path, _tool_call_id, _tool_call_path, build_skill_entry_metadata_from_read, @@ -59,9 +62,11 @@ def _stamp_task_exception_status(message: ToolMessage, *, tool_name: str, error: class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): """Convert tool exceptions into error ToolMessages so the run can continue.""" - def __init__(self, *, app_config: AppConfig | None = None) -> None: + def __init__(self, *, app_config: AppConfig | None = None, skill_authorization=None, user_id: str | None = None) -> None: super().__init__() self._app_config = app_config + self._skill_authorization = skill_authorization + self._user_id = user_id if app_config is None: self._skill_read_tool_names = frozenset(DEFAULT_SKILL_FILE_READ_TOOL_NAMES) self._skills_root = DEFAULT_SKILLS_CONTAINER_PATH @@ -69,6 +74,40 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): self._skill_read_tool_names = frozenset(app_config.summarization.skill_file_read_tool_names) self._skills_root = app_config.skills.container_path + def _storage(self): + # Same resolution order as SkillActivationMiddleware/SkillToolPolicyMiddleware: + # the registry must be the user-scoped one when a user context exists, + # or per-user custom skills would never resolve to their Skill.name. + from deerflow.skills.storage import get_or_new_skill_storage, get_or_new_user_skill_storage + + if self._user_id is not None: + return get_or_new_user_skill_storage(self._user_id, app_config=self._app_config) + if self._app_config is not None: + return get_or_new_skill_storage(app_config=self._app_config) + return get_or_new_skill_storage() + + def _canonical_skill_name(self, skill_md_path: str) -> str: + """Skill name the authorization layers speak for this SKILL.md path. + + Resolves the container path against the live (user-scoped) registry so + the ``skill:activate`` target is the declared ``Skill.name`` — the + directory name and the declared name differ for bundled skills (e.g. + ``skills/public/vercel-deploy-claimable`` declares ``vercel-deploy``), + and Layer 1 / slash / describe all authorize the declared name. + Blocking (skill-tree read): sync handlers and worker threads only. + Falls back to the path-derived name when the registry can't resolve + the path (uninstalled skill) — downstream consumers skip those paths + against the same registry anyway. + """ + from deerflow.skills.container_registry import build_container_path_registry, canonical_skill_name + + try: + registry = build_container_path_registry(self._storage()) + except Exception: + logger.warning("Failed to load the skill registry while resolving a skill read", exc_info=True) + return _skill_name_from_path(skill_md_path) + return canonical_skill_name(registry, skill_md_path) or _skill_name_from_path(skill_md_path) + def _build_error_message(self, request: ToolCallRequest, exc: Exception) -> ToolMessage: tool_name = str(request.tool_call.get("name") or "unknown_tool") tool_call_id = str(request.tool_call.get("id") or _MISSING_TOOL_CALL_ID) @@ -96,7 +135,19 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): request: ToolCallRequest, *, tool_name: str, + activation_allowed: bool | None = None, + skill_name: str | None = None, ) -> ToolMessage: + """Stamp the skill-read metadata for one completed read tool result. + + *activation_allowed* carries a ``skill:activate`` decision already + resolved by the caller — the async path awaits ``aauthorize()`` on the + event loop before calling this. ``None`` (the sync-path default) + resolves the decision here with the synchronous ``authorize()``. + *skill_name* is the already-canonicalized authorization target (the + async caller resolves it in a worker thread); when omitted, the sync + path resolves it here. + """ if tool_name not in self._skill_read_tool_names: return message if getattr(message, "status", "success") == "error": @@ -113,6 +164,34 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): entry = build_skill_entry_metadata_from_read(path, content, skills_root=self._skills_root) if entry is None: return message + # Phase 3: a completed SKILL.md read only *activates* a skill (durable + # skill_context entry, allowed-tools policy, autonomous secret binding) + # when the action-scoped ``skill:activate`` decision allows it. The + # catalog/visibility layer (Layer 1) may expose the file itself — the + # action decision is what gates the autonomous activation path the + # model drives through describe_skill + read_file. A denied read gets + # the denial marker instead of entry metadata, so extract_skills skips + # it without the "missing skill read metadata" warning. + if self._skill_authorization is not None: + if skill_name is None: + skill_name = self._canonical_skill_name(entry["path"]) + if activation_allowed is None: + from deerflow.authz.skill_filter import skill_activation_allowed + + activation_allowed = skill_activation_allowed(self._skill_authorization, skill_name) + elif activation_allowed is None: + # Authorization disabled — visibility (Layer 1) already decided + # membership, so the read activates unconditionally. + activation_allowed = True + if not activation_allowed: + logger.info( + "Skill file read for '%s' did not activate it: skill:activate denied", + skill_name or _skill_name_from_path(entry["path"]), + ) + existing = dict(message.additional_kwargs or {}) + existing[SKILL_CONTEXT_DENIED_KEY] = True + message.additional_kwargs = existing + return message existing = dict(message.additional_kwargs or {}) existing[SKILL_CONTEXT_ENTRY_KEY] = dict(entry) args = request.tool_call.get("args") or {} @@ -127,6 +206,74 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): message.additional_kwargs = existing return message + async def _amaybe_stamp(self, result: ToolMessage | Command, request: ToolCallRequest) -> ToolMessage | Command: + """Async twin of ``_maybe_stamp`` for ``awrap_tool_call``. + + Mirrors the sync structure (plain ToolMessage or Command-carried + results, tool_call_id matching, untrusted tool-returned kwargs + scrubbed before stamping) while resolving the canonical authorization + target and the ``skill:activate`` decision off the event loop in the + right order: the registry lookup (skill-tree read) goes to a worker + thread, the provider call is awaited on the loop with + ``aauthorize()`` — a synchronous ``authorize()`` here would hand + loop-affine providers the wrong API. + """ + tool_name = str(request.tool_call.get("name") or "") + tool_call_id = _tool_call_id(request.tool_call) + + async def stamp(message: ToolMessage) -> None: + # Tool-returned kwargs are untrusted. Only this middleware may add + # skill evidence after checking the configured producer and path. + existing = dict(message.additional_kwargs or {}) + existing.pop(SKILL_CONTEXT_ENTRY_KEY, None) + existing.pop(SKILL_USAGE_KEY, None) + message.additional_kwargs = existing + if tool_call_id is None or str(message.tool_call_id) == tool_call_id: + allowed: bool | None = None + skill_name: str | None = None + probe = self._resolve_skill_read_entry(message, request, tool_name=tool_name) + if probe is not None and self._skill_authorization is not None: + from deerflow.authz.skill_filter import skill_activation_allowed_async + + skill_name = await asyncio.to_thread(self._canonical_skill_name, probe["path"]) + allowed = await skill_activation_allowed_async(self._skill_authorization, skill_name) + self._stamp_skill_read_metadata(message, request, tool_name=tool_name, activation_allowed=allowed, skill_name=skill_name) + + if isinstance(result, ToolMessage): + await stamp(result) + return result + update = getattr(result, "update", None) + if not isinstance(update, dict): + return result + messages = update.get("messages") + if isinstance(messages, ToolMessage): + await stamp(messages) + elif isinstance(messages, (list, tuple)): + for message in messages: + if isinstance(message, ToolMessage): + await stamp(message) + return result + + def _resolve_skill_read_entry( + self, + message: ToolMessage, + request: ToolCallRequest, + *, + tool_name: str, + ) -> dict | None: + """Resolve the skill_context entry a completed read would stamp, if any.""" + if tool_name not in self._skill_read_tool_names: + return None + if getattr(message, "status", "success") == "error": + return None + content = message.content if isinstance(message.content, str) else None + if content is None: + return None + path = _tool_call_path(request.tool_call) + if path is None: + return None + return build_skill_entry_metadata_from_read(path, content, skills_root=self._skills_root) + def _maybe_stamp(self, result: ToolMessage | Command, request: ToolCallRequest) -> ToolMessage | Command: """Apply producer-bound metadata for tool results that need it.""" tool_name = str(request.tool_call.get("name") or "") @@ -190,7 +337,7 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]): except Exception as exc: logger.exception("Tool execution failed (async): name=%s id=%s", request.tool_call.get("name"), request.tool_call.get("id")) return self._build_error_message(request, exc) - return self._maybe_stamp( + return await self._amaybe_stamp( normalize_tool_result(result, tool_call_id=str(request.tool_call.get("id") or "")), request, ) @@ -207,6 +354,8 @@ def _build_runtime_middlewares( authorization_infrastructure_tool_names: frozenset[str] = frozenset(), available_skills: set[str] | None = None, owns_agent_skill_projection: bool = True, + skill_authorization=None, + user_id: str | None = None, ) -> list[AgentMiddleware]: """Build shared base middlewares for agent execution.""" from deerflow.agents.middlewares.input_sanitization_middleware import InputSanitizationMiddleware @@ -362,7 +511,7 @@ def _build_runtime_middlewares( tail.append(ToolProgressMiddleware.from_config(tool_progress_config)) - tail.append(ToolErrorHandlingMiddleware(app_config=app_config)) + tail.append(ToolErrorHandlingMiddleware(app_config=app_config, skill_authorization=skill_authorization, user_id=user_id)) # Artifact capture is a `before_model` hook that reads state messages, so # its position in the tool-execution wrap chain is functionally irrelevant: # it always sees the normalized results stored in state, and error results @@ -393,6 +542,8 @@ def build_lead_runtime_middlewares( deferred_setup: "DeferredToolSetup | None" = None, available_skills: set[str] | None = None, owns_agent_skill_projection: bool = True, + skill_authorization=None, + user_id: str | None = None, ) -> list[AgentMiddleware]: """Middlewares shared by lead agent runtime before lead-only middlewares.""" return _build_runtime_middlewares( @@ -406,6 +557,8 @@ def build_lead_runtime_middlewares( authorization_provider=authorization_provider, available_skills=available_skills, owns_agent_skill_projection=owns_agent_skill_projection, + skill_authorization=skill_authorization, + user_id=user_id, authorization_infrastructure_tool_names=(frozenset({deferred_setup.tool_search_tool.name}) if authorization_provider is not None and deferred_setup is not None and deferred_setup.tool_search_tool is not None else frozenset()), ) @@ -421,6 +574,7 @@ def build_subagent_runtime_middlewares( available_skills: set[str] | None = None, user_id: str | None = None, authorization_provider=None, + skill_authorization=None, extensions=None, ) -> list[AgentMiddleware]: """Middlewares shared by subagent runtime before subagent-only middlewares.""" @@ -444,6 +598,8 @@ def build_subagent_runtime_middlewares( authorization_provider=authorization_provider, authorization_infrastructure_tool_names=(frozenset({deferred_setup.tool_search_tool.name}) if authorization_provider is not None and deferred_setup is not None and deferred_setup.tool_search_tool is not None else frozenset()), owns_agent_skill_projection=False, + skill_authorization=skill_authorization, + user_id=user_id, ) # Enabled/configured skills are discoverable metadata, not automatically @@ -460,6 +616,7 @@ def build_subagent_runtime_middlewares( app_config=app_config, user_id=user_id, slash_source_owner_token=slash_source_owner_token, + skill_authorization=skill_authorization, ) ) if deferred_setup is not None and deferred_setup.deferred_names: @@ -472,6 +629,9 @@ def build_subagent_runtime_middlewares( app_config=app_config, user_id=user_id, slash_source_owner_token=slash_source_owner_token, + # Persisted skill_context entries are re-authorized against the + # skill:activate decision before their allowed-tools apply. + skill_authorization=skill_authorization, ) ) @@ -577,6 +737,11 @@ def build_subagent_runtime_middlewares( skill_file_read_tool_names=app_config.summarization.skill_file_read_tool_names, inject_tool_artifacts=app_config.tool_artifacts.enabled and app_config.tool_artifacts.inject_model_context, pii_redaction_config=getattr(app_config, "pii_redaction", None), + # The rendered "Active skills" reminder hides entries the provider + # denies; decisions are published per step by the activation + # middleware under the shared chain token. + skill_authorization=skill_authorization, + entry_decisions_owner_token=slash_source_owner_token, ) ) diff --git a/backend/packages/harness/deerflow/authz/__init__.py b/backend/packages/harness/deerflow/authz/__init__.py index 19d494ec31..df5ada60e8 100644 --- a/backend/packages/harness/deerflow/authz/__init__.py +++ b/backend/packages/harness/deerflow/authz/__init__.py @@ -1,7 +1,7 @@ """Pluggable fine-grained authorization (resource-level RBAC and beyond).""" from deerflow.authz.adapter import GuardrailAuthorizationAdapter -from deerflow.authz.enforcement import filter_tools_by_authorization +from deerflow.authz.enforcement import filter_resources_by_authorization, filter_tools_by_authorization from deerflow.authz.plugin_authz import ( PluginAuthorizationError, aenforce_plugin_action, @@ -23,6 +23,7 @@ from deerflow.authz.provider import AuthorizationProvider, AuthzDecision, AuthzR from deerflow.authz.rbac import RbacAuthorizationProvider from deerflow.authz.runtime import resolve_authorization_provider from deerflow.authz.sandbox_authz import authorize_sandbox_execution +from deerflow.authz.skill_filter import filter_available_skills_by_authorization from deerflow.authz.tool_filter import apply_tool_authorization __all__ = [ @@ -45,6 +46,8 @@ __all__ = [ "build_principal_from_context", "enforce_plugin_action", "enforce_plugin_management", + "filter_available_skills_by_authorization", + "filter_resources_by_authorization", "filter_tools_by_authorization", "normalize_authz_attributes", "plugin_action_target", diff --git a/backend/packages/harness/deerflow/authz/enforcement.py b/backend/packages/harness/deerflow/authz/enforcement.py index 93714c4d63..1f44e9e474 100644 --- a/backend/packages/harness/deerflow/authz/enforcement.py +++ b/backend/packages/harness/deerflow/authz/enforcement.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging from collections.abc import Sequence +from typing import Protocol from langchain_core.tools import BaseTool @@ -12,6 +13,12 @@ from deerflow.authz.provider import AuthorizationProvider, Principal logger = logging.getLogger(__name__) +class _Named(Protocol): + """Minimal protocol for resources identified by a ``name`` attribute.""" + + name: str + + def filter_tools_by_authorization( tools: Sequence[BaseTool], *, @@ -40,3 +47,36 @@ def filter_tools_by_authorization( allowed_names = set(allowed) return [tool for tool in original_tools if tool.name in allowed_names] + + +def filter_resources_by_authorization( + resources: Sequence[_Named], + *, + resource_type: str, + provider: AuthorizationProvider | None, + principal: Principal, + fail_closed: bool, +) -> list[_Named]: + """Return the policy-visible subset of *resources* without changing order. + + Generic batch filter for any resource identified by a ``name`` attribute + (skills, models, etc.). Mirrors ``filter_tools_by_authorization`` but is + not coupled to ``BaseTool``. Provider errors and malformed filter results + deny every resource when ``fail_closed`` is true; an explicitly configured + fail-open policy preserves the original set. + """ + original = list(resources) + if provider is None: + return original + + candidates = [r.name for r in original] + try: + allowed = provider.filter_resources(principal, resource_type, candidates) + if not isinstance(allowed, list) or any(not isinstance(name, str) for name in allowed): + raise TypeError("AuthorizationProvider.filter_resources must return list[str]") + except Exception: + logger.exception("Authorization provider failed while filtering %s", resource_type) + return [] if fail_closed else original + + allowed_names = set(allowed) + return [r for r in original if r.name in allowed_names] diff --git a/backend/packages/harness/deerflow/authz/skill_filter.py b/backend/packages/harness/deerflow/authz/skill_filter.py new file mode 100644 index 0000000000..2bb72a79aa --- /dev/null +++ b/backend/packages/harness/deerflow/authz/skill_filter.py @@ -0,0 +1,237 @@ +"""Convenience wrapper for Layer 1 skill authorization filtering. + +Combines provider resolution, Principal construction, and skill filtering into +a single call so the assembly paths (lead agent, subagent executor) stay +one-liners. Mirrors ``apply_tool_authorization`` from ``tool_filter.py``. +""" + +from __future__ import annotations + +import logging +from collections.abc import Mapping +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from deerflow.authz.principal import build_principal_from_context +from deerflow.authz.provider import AuthzDecision, AuthzRequest +from deerflow.authz.runtime import resolve_authorization_provider +from deerflow.config.app_config import AppConfig + +if TYPE_CHECKING: + from deerflow.authz.provider import AuthorizationProvider + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True, slots=True) +class ResolvedSkillAuthorization: + """Provider + principal resolved once per agent assembly. + + Shared by the Layer 1 visibility filter above and the runtime + ``skill:activate`` action check in ``SkillActivationMiddleware`` so both + layers see the same provider instance and identity without resolving + twice. ``fail_closed`` is the provider-error policy from config. + """ + + provider: AuthorizationProvider + principal: Any + fail_closed: bool + + +def skill_activation_allowed( + authorization: ResolvedSkillAuthorization | None, + skill_name: str, +) -> bool: + """Action-scoped ``skill:activate`` decision for one skill name. + + Shared by every runtime activation gate — explicit slash activation + (``SkillActivationMiddleware``), ``describe_skill``, and the skill-file + load that records ``skill_context`` — so all paths apply the same decision + and the same provider-error policy. ``_available_skills``-style visibility + sets are Layer 1 (action-agnostic ``filter_resources``); an action-aware + custom provider may expose a skill there while denying its activation, so + each activation path must additionally consult this check. ``None`` + (authorization disabled) allows — visibility already decided membership. + Mirrors ``_authorize_model_name``'s second ``authorize("model", "use")`` + check. Provider errors follow the configured fail-closed / fail-open + policy. + """ + if authorization is None: + return True + try: + decision = authorization.provider.authorize(AuthzRequest(principal=authorization.principal, resource="skill", action="activate", target=skill_name)) + if not isinstance(decision, AuthzDecision): + raise TypeError("AuthorizationProvider.authorize must return AuthzDecision") + return decision.allow + except Exception: + logger.warning("Authorization provider failed while checking skill:activate for '%s'", skill_name, exc_info=True) + return not authorization.fail_closed + + +async def skill_activation_allowed_async( + authorization: ResolvedSkillAuthorization | None, + skill_name: str, +) -> bool: + """Async counterpart of :func:`skill_activation_allowed`. + + Uses ``provider.aauthorize()`` so loop-affine custom providers (clients + bound to the running event loop) receive the async API on async execution + paths, mirroring ``authorize_sandbox_execution_async``. Same decision + semantics and fail-closed / fail-open policy as the sync version. + """ + if authorization is None: + return True + try: + decision = await authorization.provider.aauthorize(AuthzRequest(principal=authorization.principal, resource="skill", action="activate", target=skill_name)) + if not isinstance(decision, AuthzDecision): + raise TypeError("AuthorizationProvider.aauthorize must return AuthzDecision") + return decision.allow + except Exception: + logger.warning("Authorization provider failed while checking skill:activate for '%s'", skill_name, exc_info=True) + return not authorization.fail_closed + + +def resolve_skill_authorization( + context: Mapping[str, Any], + app_config: AppConfig, +) -> ResolvedSkillAuthorization | None: + """Resolve the skill authorization context, or ``None`` when disabled. + + ``None`` means "authorization is disabled or no provider is configured" — + callers treat that as "no extra enforcement" (the pre-authorization + behavior). Mock-friendly in the same style as the filter below: a config + object without a real ``AuthorizationConfig`` also yields ``None``. + """ + authz_config = getattr(app_config, "authorization", None) + if authz_config is None or getattr(authz_config, "enabled", None) is not True: + return None + + provider = resolve_authorization_provider(authz_config) + if provider is None: + return None + + principal = build_principal_from_context(context, default_role=authz_config.default_role) + return ResolvedSkillAuthorization( + provider=provider, + principal=principal, + fail_closed=bool(getattr(authz_config, "fail_closed", False)), + ) + + +def filter_available_skills_by_authorization( + available_skills: set[str] | None, + *, + context: Mapping[str, Any], + app_config: AppConfig, + user_id: str | None = None, + candidate_skill_names: list[str] | None = None, + authorization: ResolvedSkillAuthorization | None = None, +) -> set[str] | None: + """Filter a skill-name allowlist by the provider's ``"skill"`` policy. + + This is the Layer 1 entry point for the lead-agent and subagent assembly + paths. It operates on the ``available_skills`` name set (the agent-config + allowlist) rather than on loaded ``Skill`` objects, so it runs before any + disk I/O and constrains both the catalog (``describe_skill``) and the + slash-activation middleware (which checks the same set). + + - ``available_skills=None`` means "no agent-level allowlist" (all enabled + skills). When authorization is enabled, this resolves the full enabled + skill set from config and filters it; the result is a constrained set + (or ``None`` if authorization is disabled). + - ``available_skills=set()`` (empty) means "no skills for this agent" and + is preserved as-is (the agent already allows none). + + *user_id* threads the per-user identity into the candidate resolution so + per-user custom skills (``UserScopedSkillStorage``) are included in the + universe the provider filters — matching what the activation path loads. + When omitted, only the process-global (public) skills are candidates. + + *candidate_skill_names* lets a caller that has already loaded skills from + disk (e.g. the subagent executor) pass the pre-resolved name list, avoiding + a redundant ``load_skills`` call inside ``_all_configured_skill_names``. + Only consulted when ``available_skills is None``. + + When ``authorization.enabled`` is false, returns *available_skills* + unchanged (no-op). + """ + # Guard against Mock/SimpleNamespace app_config objects in tests that + # don't carry a real AuthorizationConfig. getattr avoids AttributeError + # and the ``is not True`` identity check avoids truthy Mock attributes. + if authorization is None: + authorization = resolve_skill_authorization(context, app_config) + if authorization is None: + return available_skills + fail_closed = authorization.fail_closed + + principal = authorization.principal + + # If the agent already allows no skills, nothing to filter. + if available_skills is not None and len(available_skills) == 0: + return available_skills + + # Resolve the candidate set: the agent allowlist (if set), or all known + # skill names from config (so authorization constrains the "no allowlist" + # case too). When ``available_skills is None`` and the configured-skill + # resolution fails (storage I/O error), we must NOT silently return ``None`` + # — that would bypass authorization entirely (every skill becomes + # activatable/loadable). Instead we fall through to the same fail-closed / + # fail-open handling as a provider error below. + if available_skills is not None: + candidates = sorted(available_skills) + elif candidate_skill_names is not None: + # Caller already loaded skills from disk (e.g. subagent executor) — + # reuse the names instead of calling load_skills again. + candidates = candidate_skill_names + else: + try: + candidates = _all_configured_skill_names(app_config, user_id=user_id) + except Exception: + logger.warning("Could not resolve configured skill names for authorization filtering", exc_info=True) + return set() if fail_closed else available_skills + + if not candidates: + # Genuinely no skills configured (or agent allowlist is empty after + # filtering) — ``None`` here is safe because there is nothing to authorize. + return available_skills + + try: + allowed = authorization.provider.filter_resources(principal, "skill", candidates) + if not isinstance(allowed, list) or any(not isinstance(n, str) for n in allowed): + raise TypeError("AuthorizationProvider.filter_resources must return list[str]") + except Exception: + logger.warning("Authorization provider failed while filtering available skills", exc_info=True) + return set() if fail_closed else available_skills + + # Defensive intersection with the candidate set: the provider contract says + # results must be a subset of candidates, but we only validated list[str]. + # A buggy custom provider could otherwise inject extra skill names and + # expand an explicit agent allowlist. Mirrors filter_resources_by_authorization. + allowed_names = set(allowed) + return {name for name in candidates if name in allowed_names} + + +def _all_configured_skill_names(app_config: AppConfig, *, user_id: str | None = None) -> list[str]: + """Return all configured skill names (public + per-user custom). + + This is only needed when ``available_skills`` is ``None`` (no agent-level + allowlist) and authorization is enabled — we need the full candidate set + to filter. Returns an empty list if no skills are configured. Raises on + storage/I/O errors so the caller can apply fail-closed / fail-open semantics + (a swallowed error here would silently bypass authorization). + + When *user_id* is provided, uses the per-user ``UserScopedSkillStorage`` so + custom skills under ``{base}/users/{user_id}/skills/custom/`` are included — + matching the universe the activation path loads. Without it, falls back to + the process-global storage (public skills only). + """ + if user_id is not None: + from deerflow.skills.storage import get_or_new_user_skill_storage + + storage = get_or_new_user_skill_storage(user_id, app_config=app_config) + else: + from deerflow.skills.storage import get_or_new_skill_storage + + storage = get_or_new_skill_storage(app_config=app_config) + skills = storage.load_skills(enabled_only=True) + return [s.name for s in skills] diff --git a/backend/packages/harness/deerflow/client.py b/backend/packages/harness/deerflow/client.py index fd6a445fb7..64b936130f 100644 --- a/backend/packages/harness/deerflow/client.py +++ b/backend/packages/harness/deerflow/client.py @@ -380,6 +380,45 @@ class DeerFlowClient: if model_name is None and self._app_config.models: model_name = self._app_config.models[0].name model_name = _authorize_model_name(model_name, context=cfg, app_config=self._app_config) + # Phase 3: enforce skill authorization (Layer 1) on the embedded path + # too, mirroring ``_make_lead_agent`` (agent.py:675). Without this, a + # caller building the agent via ``DeerFlowClient(available_skills=...)`` + # bypasses the role's ``skills`` policy: ``SkillActivationMiddleware`` + # and the catalog/prompt would see the unfiltered set. ``self._available_skills`` + # is left untouched (it feeds the cache key at line 282); the filtered + # result is a local used for assembly only. + from deerflow.authz.skill_filter import filter_available_skills_by_authorization, resolve_skill_authorization + + # One effective identity for the whole skill surface: the filter's + # candidate universe, the candidate pre-load's cache bucket, and the + # middleware's user-scoped storage must all resolve the same user, or + # per-user custom skills visible to the middleware would be missing + # from the filtered set (and vice versa). Resolved before anything + # consumes a user id below. + effective_user_id = cfg.get("user_id") or get_effective_user_id() + + skill_authorization = resolve_skill_authorization(cfg, self._app_config) + # Resolve the filter's candidate names through the cached catalog + # loader (same source and same user bucket as ``skills_list`` below) + # instead of letting the filter rescan storage on every build; only + # relevant when no agent-level allowlist is set. + candidate_skill_names = None + if self._available_skills is None and skill_authorization is not None: + try: + candidate_skill_names = [s.name for s in get_enabled_skills_for_config(self._app_config, user_id=effective_user_id)] + except Exception: + logger.warning("Failed to pre-load enabled skills for authorization candidates", exc_info=True) + # Leave None: the filter resolves candidates itself and applies + # its fail-closed semantics when that resolution also fails. + + available_skills = filter_available_skills_by_authorization( + self._available_skills, + context=cfg, + app_config=self._app_config, + user_id=effective_user_id, + candidate_skill_names=candidate_skill_names, + authorization=skill_authorization, + ) subagent_enabled = cfg.get("subagent_enabled", False) from deerflow.config.subagents_config import effective_subagent_concurrency, effective_total_subagents_per_run @@ -403,13 +442,16 @@ class DeerFlowClient: # Add framework-provided tools before authorization so Layer 1 sees # every capability that can become model-visible. - skills_list = get_enabled_skills_for_config(self._app_config) - if self._available_skills is not None: - skills_list = [s for s in skills_list if s.name in self._available_skills] + # Same effective user as the filter/candidates above so the catalog + # shares the per-user cache bucket and the same skill universe. + skills_list = get_enabled_skills_for_config(self._app_config, user_id=effective_user_id) + if available_skills is not None: + skills_list = [s for s in skills_list if s.name in available_skills] skill_setup = build_skill_search_setup( skills_list, enabled=self._app_config.skills.deferred_discovery, container_base_path=self._app_config.skills.container_path, + skill_authorization=skill_authorization, ) from deerflow.agents.task_continuity.tools import append_task_continuity_tools @@ -450,7 +492,7 @@ class DeerFlowClient: config, model_name=model_name, agent_name=self._agent_name, - available_skills=self._available_skills, + available_skills=available_skills, memory_enabled=memory_enabled, custom_middlewares=self._middlewares, app_config=self._app_config, @@ -458,6 +500,7 @@ class DeerFlowClient: mcp_routing_middleware=mcp_routing_middleware, user_id=effective_user_id, authorization_provider=_authz_provider, + skill_authorization=skill_authorization, subagent_execution_capacity=subagent_execution_capacity, ), self._checkpoint_channel_mode, @@ -468,7 +511,7 @@ class DeerFlowClient: max_concurrent_subagents=max_concurrent_subagents, max_total_subagents=max_total_subagents, agent_name=self._agent_name, - available_skills=self._available_skills, + available_skills=available_skills, app_config=self._app_config, deferred_names=deferred_setup.deferred_names, mcp_routing_hints_section=mcp_routing_hints_section, diff --git a/backend/packages/harness/deerflow/runtime/secret_context.py b/backend/packages/harness/deerflow/runtime/secret_context.py index 7106553def..0fd5cf1e14 100644 --- a/backend/packages/harness/deerflow/runtime/secret_context.py +++ b/backend/packages/harness/deerflow/runtime/secret_context.py @@ -103,6 +103,44 @@ def read_slash_skill_source_path(context: Any, *, owner_token: str) -> str | Non return path if isinstance(path, str) and path else None +# Reserved run-context key holding the per-model-step ``skill:activate`` +# decisions for persisted ``skill_context`` entry paths, keyed by normalized +# container SKILL.md path. Published by the skill-activation middleware (async +# hook: reused from the prepass snapshot; sync hook: one scan + sync +# authorize) and consumed by the durable-context renderer so the model-visible +# "Active skills" reminder stops advertising skills the provider now denies. +# Names only — never secret values. Authenticated by the chain-local owner +# token, same contract as the slash source above. +_SKILL_ENTRY_DECISIONS_KEY = "__skill_entry_activation_decisions" + + +def write_skill_entry_decisions(context: Any, decisions: dict[str, bool], *, owner_token: str) -> None: + """Publish per-step entry activation decisions for downstream renderers.""" + if isinstance(context, dict) and isinstance(decisions, dict) and isinstance(owner_token, str) and owner_token: + context[_SKILL_ENTRY_DECISIONS_KEY] = {"decisions": dict(decisions), "owner_token": owner_token} + + +def read_skill_entry_decisions(context: Any, *, owner_token: str) -> dict[str, bool] | None: + """Return the authenticated entry decisions, or ``None`` when unpublished. + + ``None`` means "no decision published this step" (authorization disabled, + no entries, or a foreign/malformed carrier) — consumers then render the + entries unchanged, matching the absent-is-permissive contract of the + other run-context carriers. + """ + if not isinstance(context, dict): + return None + carrier = context.get(_SKILL_ENTRY_DECISIONS_KEY) + if not isinstance(carrier, dict): + return None + if not isinstance(owner_token, str) or not owner_token or carrier.get("owner_token") != owner_token: + return None + decisions = carrier.get("decisions") + if not isinstance(decisions, dict) or not all(isinstance(k, str) and isinstance(v, bool) for k, v in decisions.items()): + return None + return decisions + + # Private run-context keys the skill-activation middleware uses to carry secret # bindings across a run. Only ``secrets`` / ``__active_skill_secrets`` hold # secret values; the slash source holds a middleware-chain owner token, while @@ -131,6 +169,7 @@ REDACTED_CONTEXT_KEYS = frozenset( _SECRETS_BINDING_AUDIT_KEY, _SLASH_SKILL_ACTIVATION_RUN_KEY, SKILL_TOOL_POLICY_DECISION_CONTEXT_KEY, + _SKILL_ENTRY_DECISIONS_KEY, } ) diff --git a/backend/packages/harness/deerflow/skills/container_registry.py b/backend/packages/harness/deerflow/skills/container_registry.py new file mode 100644 index 0000000000..b6269e6ad0 --- /dev/null +++ b/backend/packages/harness/deerflow/skills/container_registry.py @@ -0,0 +1,40 @@ +"""Map container SKILL.md paths to the live registry's canonical skills. + +The directory a skill lives in (``relative_path``, e.g. +``vercel-deploy-claimable``) and the name its SKILL.md declares +(``Skill.name``, e.g. ``vercel-deploy``) are deliberately independent: the +authorization layers (Layer 1 filtering, slash activation, ``describe_skill``) +all speak the declared ``Skill.name``, so any path-derived lookup must be +resolved through this registry before it is compared against — or sent to — an +authorization decision. +""" + +from __future__ import annotations + +import posixpath +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from deerflow.skills.types import Skill + + +def build_container_path_registry(storage) -> dict[str, Skill]: + """Build the live registry keyed by normalized container SKILL.md path. + + Blocking (full skill-tree read): call from sync handlers or a worker + thread, never the event loop. ``enabled_only=False`` on purpose — the + activation decision is authorization's to make, not the enabled flag's. + """ + container_root = storage.get_container_root() + return {posixpath.normpath(skill.get_container_file_path(container_root)): skill for skill in storage.load_skills(enabled_only=False)} + + +def canonical_skill_name(registry: dict[str, Skill], skill_md_path: str) -> str | None: + """Resolve a container SKILL.md path to the registry skill's declared name. + + ``None`` for a path the registry does not know (uninstalled, forged, or + failed to parse) — callers decide their own fallback, typically the + path-derived directory name for display or a skip for policy. + """ + skill = registry.get(posixpath.normpath(skill_md_path)) + return skill.name if skill is not None else None diff --git a/backend/packages/harness/deerflow/skills/describe.py b/backend/packages/harness/deerflow/skills/describe.py index 4417dbf0e3..3d3488b99e 100644 --- a/backend/packages/harness/deerflow/skills/describe.py +++ b/backend/packages/harness/deerflow/skills/describe.py @@ -17,7 +17,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Annotated from langchain_core.messages import ToolMessage -from langchain_core.tools import InjectedToolCallId, tool +from langchain_core.tools import InjectedToolCallId from langgraph.types import Command if TYPE_CHECKING: @@ -53,15 +53,70 @@ def build_describe_skill_tool( catalog: SkillCatalog, *, container_base_path: str = DEFAULT_SKILLS_CONTAINER_PATH, + skill_authorization=None, ) -> BaseTool: """Build the ``describe_skill`` tool as a closure over *catalog*. - The returned tool is a plain ``@tool``-decorated function that searches the - catalog and returns a ``Command`` wrapping a ``ToolMessage``. No graph state - mutation is needed (unlike ``tool_search`` which promotes deferred tools). + The returned tool carries both a sync function and a coroutine: sync + invocation (``invoke``, e.g. tests and sync chains) gates matches with the + provider's synchronous ``authorize()``, while async invocation (``ainvoke``, + the path async agent execution takes) awaits ``aauthorize()`` on the event + loop — loop-affine custom providers must never see the sync API. No graph + state mutation is needed (unlike ``tool_search`` which promotes deferred + tools). + + *skill_authorization* (a ``ResolvedSkillAuthorization`` from + ``deerflow.authz.skill_filter``) gates describe results on the action-scoped + ``skill:activate`` decision: the catalog holds the Layer 1 visibility set + (action-agnostic), so an action-aware provider may list a skill the model + must still not be able to load. Denied skills are omitted from the output + (indistinguishable from a non-match, so the reason is not leaked) with the + provider's configured fail-closed / fail-open policy on errors. ``None`` + (authorization disabled) describes everything in the catalog. """ - @tool + def _log_omissions(omitted: int) -> None: + if omitted: + logger.info("describe_skill: omitted %d skill(s) denied by the skill:activate policy", omitted) + + def _filter_activation_denied(matched: list) -> list: + if skill_authorization is None: + return matched + from deerflow.authz.skill_filter import skill_activation_allowed + + kept = [s for s in matched if skill_activation_allowed(skill_authorization, s.name)] + _log_omissions(len(matched) - len(kept)) + return kept + + async def _afilter_activation_denied(matched: list) -> list: + if skill_authorization is None: + return matched + from deerflow.authz.skill_filter import skill_activation_allowed_async + + kept = [] + for s in matched: + if await skill_activation_allowed_async(skill_authorization, s.name): + kept.append(s) + _log_omissions(len(matched) - len(kept)) + return kept + + def _command(matched: list, name: str, tool_call_id: str) -> Command: + if not matched: + content = f"No skills matched: {name}" + else: + content = _render_skill_metadata(matched, container_base_path) + return Command( + update={ + "messages": [ + ToolMessage( + content=content, + tool_call_id=tool_call_id, + name="describe_skill", + ) + ], + } + ) + def describe_skill( name: str, tool_call_id: Annotated[str, InjectedToolCallId], @@ -79,25 +134,23 @@ def build_describe_skill_tool( - "chart visualization" -- keyword search, best matches (up to 5) - "+podcast gen" -- require "podcast" in the name, rank by remaining terms (up to 5) """ - matched = catalog.search(name) - if not matched: - content = f"No skills matched: {name}" - else: - content = _render_skill_metadata(matched, container_base_path) + return _command(_filter_activation_denied(catalog.search(name)), name, tool_call_id) - return Command( - update={ - "messages": [ - ToolMessage( - content=content, - tool_call_id=tool_call_id, - name="describe_skill", - ) - ], - } - ) + async def adescribe_skill( + name: str, + tool_call_id: Annotated[str, InjectedToolCallId], + ) -> Command: + return _command(await _afilter_activation_denied(catalog.search(name)), name, tool_call_id) - return describe_skill + from langchain_core.tools import StructuredTool + + return StructuredTool.from_function( + func=describe_skill, + coroutine=adescribe_skill, + name="describe_skill", + description=describe_skill.__doc__, + parse_docstring=False, + ) def build_skill_search_setup( @@ -105,12 +158,15 @@ def build_skill_search_setup( *, enabled: bool, container_base_path: str = DEFAULT_SKILLS_CONTAINER_PATH, + skill_authorization=None, ) -> SkillSearchSetup: """Build the skill search setup from a filtered skill list. Mirrors ``build_deferred_tool_setup`` from ``tool_search.py``. Returns an empty setup when *enabled* is ``False`` or *skills* is empty. + *skill_authorization* is forwarded to the describe tool (see + ``build_describe_skill_tool``). """ if not enabled or not skills: return SkillSearchSetup(None, frozenset()) @@ -120,6 +176,7 @@ def build_skill_search_setup( describe_skill_tool=build_describe_skill_tool( catalog, container_base_path=container_base_path, + skill_authorization=skill_authorization, ), skill_names=catalog.names, ) diff --git a/backend/packages/harness/deerflow/subagents/executor.py b/backend/packages/harness/deerflow/subagents/executor.py index eecb1a4410..29f52cd018 100644 --- a/backend/packages/harness/deerflow/subagents/executor.py +++ b/backend/packages/harness/deerflow/subagents/executor.py @@ -941,6 +941,11 @@ class SubagentExecutor: # not just the first — because the v2 contract advertises more than one # cap reason. self._stop_reason_middlewares: list[Any] = [] + # The one skill-authorization context for this execution, resolved + # lazily by ``_resolve_skill_authorization`` and shared by the Layer 1 + # filter and every runtime ``skill:activate`` check (activation + # middleware, describe gate, skill-read stamping). + self._skill_authorization: Any | None = None # LangGraph super-step budget that buys ``config.max_turns`` turns, # resolved in ``_create_agent`` once the middleware chain — and with it # the compiled graph's per-turn node count — is known. Stays ``None`` @@ -1008,6 +1013,17 @@ class SubagentExecutor: authz_provider = getattr(self, "_authz_provider", None) if authz_provider is not None: middleware_kwargs["authorization_provider"] = authz_provider + # Phase 3 (Layer 2): arm the runtime ``skill:activate`` check on the + # subagent chain too — a delegated task is a plain HumanMessage, so + # task text containing /skill-name reaches the activation middleware. + # Resolved through the same memoized helper the Layer 1 filter used + # (``_resolve_skill_authorization``): provider factories are + # deliberately uncached, so resolving independently here could hand the + # runtime checks a different provider instance (and policy snapshot) + # than the filter that built ``_available_skill_names``. + skill_authorization = self._resolve_skill_authorization() + if skill_authorization is not None: + middleware_kwargs["skill_authorization"] = skill_authorization if mcp_routing_middleware is not None: middleware_kwargs["mcp_routing_middleware"] = mcp_routing_middleware middlewares = build_subagent_runtime_middlewares(**middleware_kwargs) @@ -1205,9 +1221,67 @@ class SubagentExecutor: # Filter by config.skills whitelist if self.config.skills is not None: allowed = set(self.config.skills) + else: + allowed = None + + # Phase 3: enforce skill authorization (Layer 1). Filter the skill + # allowlist by the provider's "skill" policy so denied skills never + # reach the returned list (and are never turned into tools). Pass the + # already-loaded ``all_skills`` names as candidates so we don't pay a + # second ``load_skills`` round-trip inside the filter, and the memoized + # authorization context so the filter's provider instance is the same + # one the runtime ``skill:activate`` checks on this assembly use. + from deerflow.authz.skill_filter import filter_available_skills_by_authorization + + resolved_app_config = self.app_config or get_app_config() + allowed = filter_available_skills_by_authorization( + allowed, + context=self._skill_authz_context(), + app_config=resolved_app_config, + # Same bucket convention as the middleware chain (self.user_id or + # DEFAULT_USER_ID); only consulted if candidates were not supplied. + user_id=self.user_id or DEFAULT_USER_ID, + candidate_skill_names=[s.name for s in all_skills], + authorization=self._resolve_skill_authorization(), + ) + + if allowed is not None: return [s for s in all_skills if s.name in allowed] return all_skills + def _resolve_skill_authorization(self): + """The one skill-authorization context for this execution. + + Resolved once per executor assembly (executors are built per task + dispatch) and shared by the Layer 1 filter in ``_load_skills``, the + runtime ``skill:activate`` check on the activation middleware, the + ``describe_skill`` gate, and the skill-read stamping middleware — so + every layer of this assembly sees the same provider instance and + policy snapshot. Providers are uncached by design; resolving per layer + would let a custom provider observe different snapshots for visibility + and activation within one run. + """ + resolved = getattr(self, "_skill_authorization", None) + if resolved is None: + from deerflow.authz.skill_filter import resolve_skill_authorization + + resolved = resolve_skill_authorization(self._skill_authz_context(), self._get_resolved_app_config()) + self._skill_authorization = resolved + return resolved + + def _skill_authz_context(self) -> dict[str, Any]: + """Identity mapping shared by the Layer 1 filter and the runtime + ``skill:activate`` check wired into the subagent middleware chain.""" + return { + "user_id": self.user_id, + "user_role": self.user_role, + "oauth_provider": self.oauth_provider, + "oauth_id": self.oauth_id, + "channel_user_id": self.channel_user_id, + "is_internal": self.is_internal, + "authz_attributes": self.authz_attributes, + } + async def _build_initial_state(self, task: str) -> tuple[dict[str, Any], list[BaseTool], "DeferredToolSetup"]: """Build the initial state for agent execution. @@ -1241,6 +1315,7 @@ class SubagentExecutor: skills, enabled=resolved_app_config.skills.deferred_discovery, container_base_path=resolved_app_config.skills.container_path, + skill_authorization=self._resolve_skill_authorization(), ) # Apply authorization Layer 1: filter tools before deferred assembly diff --git a/backend/tests/test_authorization_enforcement.py b/backend/tests/test_authorization_enforcement.py index eed20b6b14..c518bda60e 100644 --- a/backend/tests/test_authorization_enforcement.py +++ b/backend/tests/test_authorization_enforcement.py @@ -389,3 +389,86 @@ def test_lead_agent_filters_all_model_visible_tools_and_reuses_provider(monkeypa assert [tool.name for tool in result["tools"]] == ["safe_tool", "history_read"] assert captured["authorization_provider"] is not None + + +@pytest.mark.parametrize( + "skills_allow,deferred,expected_names", + [ + (["bootstrap"], True, frozenset({"bootstrap"})), + ([], True, frozenset()), + ([], False, None), + ], +) +def test_bootstrap_assembly_uses_filtered_skill_set(monkeypatch, skills_allow, deferred, expected_names): + """[P1 regression] The bootstrap branch must feed the authorization-filtered + set to ``build_middlewares`` and ``apply_prompt_template`` — not the raw + ``_BOOTSTRAP_SKILL_NAMES`` — and must preserve an empty ``skill_names`` + index instead of converting it to ``None`` (a None falls through to the + legacy full-metadata prompt, which reloads every enabled skill from storage + and re-advertises the policy-denied one; a raw {"bootstrap"} set re-admits + it to slash activation).""" + config = _app_config( + authorization=AuthorizationConfig( + enabled=True, + provider=AuthorizationProviderConfig( + use="deerflow.authz.rbac:RbacAuthorizationProvider", + config={"roles": {"user": {"skills": {"allow": skills_allow}}}}, + ), + ), + models=[ + ModelConfig( + name="test-model", + display_name="Test model", + use="langchain_openai:ChatOpenAI", + model="test-model", + ) + ], + ) + config.skills.deferred_discovery = deferred + + monkeypatch.setattr(lead_agent_module, "_resolve_model_name", lambda *args, **kwargs: "test-model") + monkeypatch.setattr(lead_agent_module, "create_chat_model", lambda **kwargs: object()) + monkeypatch.setattr(lead_agent_module, "create_agent", lambda **kwargs: kwargs) + monkeypatch.setattr(lead_agent_module, "build_tracing_callbacks", lambda: []) + # Emulate the real loader's allowlist filtering so the allowed case keeps + # the bootstrap skill and the denied case drops it. + monkeypatch.setattr( + lead_agent_module, + "_load_enabled_available_skills", + lambda available_skills, **kwargs: [s for s in (SimpleNamespace(name="bootstrap"), SimpleNamespace(name="other")) if available_skills is None or s.name in available_skills], + ) + # Emulate the real setup mapping (bootstrap_skills -> skill_names) without + # depending on SkillCatalog internals; the real mapping has its own tests. + monkeypatch.setattr( + lead_agent_module, + "build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace( + describe_skill_tool=None, + skill_names=frozenset(s.name for s in skills), + ), + raising=False, + ) + monkeypatch.setattr("deerflow.tools.get_available_tools", lambda **kwargs: []) + monkeypatch.setattr(lead_agent_module, "should_use_memory_tools", lambda memory_config: False) + + captured: dict[str, dict] = {} + + def _capture_middlewares(*args, **kwargs): + captured.setdefault("middlewares", kwargs) + return [] + + def _capture_prompt(**kwargs): + captured.setdefault("prompt", kwargs) + return "prompt" + + monkeypatch.setattr(lead_agent_module, "build_middlewares", _capture_middlewares) + monkeypatch.setattr(lead_agent_module, "apply_prompt_template", _capture_prompt) + + lead_agent_module._make_lead_agent({"context": {"is_bootstrap": True, "user_role": "user"}}, app_config=config) + + expected_available = {"bootstrap"} if "bootstrap" in skills_allow else set() + assert captured["middlewares"]["available_skills"] == expected_available + assert captured["prompt"]["available_skills"] == expected_available + # The empty index must survive as a set, never round-trip through None. + assert captured["prompt"]["skill_names"] == expected_names + assert captured["prompt"]["skill_names"] is not None or expected_names is None diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index e4bc7199be..302f14252b 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -1597,6 +1597,7 @@ class TestEnsureAgent: patch.object(client, "_get_tools", return_value=[safe_tool, denied_tool]), patch("deerflow.authz.tool_filter.resolve_authorization_provider", return_value=provider), patch("deerflow.agents.lead_agent.agent.resolve_authorization_provider", return_value=provider), + patch("deerflow.authz.skill_filter.resolve_authorization_provider", return_value=provider), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=None), ): client._ensure_agent(client._get_runnable_config("t1"), context={"user_role": "user"}) diff --git a/backend/tests/test_models_authorization.py b/backend/tests/test_models_authorization.py index 7ca1e1f9f3..377064c5c8 100644 --- a/backend/tests/test_models_authorization.py +++ b/backend/tests/test_models_authorization.py @@ -682,10 +682,10 @@ def _stub_client_assembly(monkeypatch) -> dict[str, str]: monkeypatch.setattr("deerflow.client.create_agent", lambda **kwargs: object()) monkeypatch.setattr("deerflow.client.build_middlewares", lambda *args, **kwargs: []) monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 - monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", lambda app_config: []) # noqa: ARG005 + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", lambda app_config, **kw: []) # noqa: ARG005 monkeypatch.setattr( "deerflow.client.build_skill_search_setup", - lambda skills, *, enabled, container_base_path: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), # noqa: ARG005 + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), # noqa: ARG005 ) monkeypatch.setattr( "deerflow.client.assemble_deferred_tools", @@ -699,11 +699,16 @@ def _stub_client_assembly(monkeypatch) -> dict[str, str]: monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") # ``apply_tool_authorization`` (called with the empty tool list above) still # resolves a provider via ``tool_filter.resolve_authorization_provider``; route - # it at an allow-all RBAC provider so the empty list stays empty. + # it at an allow-all RBAC provider so the empty list stays empty. The skill + # filter (added in Phase 3 Skills PR) resolves via ``skill_filter`` namespace. monkeypatch.setattr( "deerflow.authz.tool_filter.resolve_authorization_provider", lambda config: RbacAuthorizationProvider(roles={"user": {"tools": {"allow": "*"}}}), ) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: RbacAuthorizationProvider(roles={"user": {"skills": {"allow": "*"}}}), + ) return captured diff --git a/backend/tests/test_skill_tool_policy_middleware.py b/backend/tests/test_skill_tool_policy_middleware.py index b1f1bbc577..382f7c868a 100644 --- a/backend/tests/test_skill_tool_policy_middleware.py +++ b/backend/tests/test_skill_tool_policy_middleware.py @@ -401,7 +401,7 @@ def test_slash_activation_and_policy_compose_on_the_same_model_call(monkeypatch) editable=False, ) activation_middleware = SkillActivationMiddleware(slash_source_owner_token=_SLASH_SOURCE_OWNER_TOKEN) - monkeypatch.setattr(activation_middleware, "_resolve_activation", lambda _: _ActivationResolution(activation=activation)) + monkeypatch.setattr(activation_middleware, "_resolve_activation", lambda _, activation_decisions=None: _ActivationResolution(activation=activation)) policy_middleware = _middleware([skill]) request = ModelRequestStub( [NamedTool("task"), NamedTool("read_file"), NamedTool("review_skill_package")], diff --git a/backend/tests/test_skills_authorization.py b/backend/tests/test_skills_authorization.py new file mode 100644 index 0000000000..59b97259af --- /dev/null +++ b/backend/tests/test_skills_authorization.py @@ -0,0 +1,3082 @@ +"""Phase 3 skill-level authorization tests. + +Covers Layer 1 enforcement: +- ``filter_available_skills_by_authorization`` — the lead-agent / subagent + entry point that filters the skill-name allowlist by the provider's + ``"skill"`` policy. +- ``filter_resources_by_authorization`` — the generic batch filter. +- Subagent executor integration: ``_load_skills`` respects authorization. +""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +from deerflow.authz.enforcement import filter_resources_by_authorization +from deerflow.authz.principal import build_principal_from_context +from deerflow.authz.rbac import RbacAuthorizationProvider +from deerflow.authz.skill_filter import filter_available_skills_by_authorization +from deerflow.config.app_config import AppConfig +from deerflow.config.authorization_config import AuthorizationConfig +from deerflow.config.model_config import ModelConfig +from deerflow.config.sandbox_config import SandboxConfig + +# ── Helpers ──────────────────────────────────────────────────────────── + + +def _make_app_config() -> AppConfig: + """Build a minimal AppConfig for authorization tests.""" + return AppConfig( + models=[ModelConfig(name="gpt-4", model="gpt-4", use="langchain_openai:ChatOpenAI")], + sandbox=SandboxConfig(use="deerflow.sandbox.local:LocalSandboxProvider"), + authorization=AuthorizationConfig(), + ) + + +def _context(**overrides): + values = { + "user_id": "user-123", + "user_role": "user", + "oauth_provider": "github", + "oauth_id": "oauth-456", + "is_internal": False, + } + values.update(overrides) + return values + + +def _rbac_provider(roles: dict) -> RbacAuthorizationProvider: + return RbacAuthorizationProvider(roles=roles) + + +def _enable_authz(app_config: AppConfig, *, fail_closed: bool = True, default_role: str = "user") -> None: + app_config.authorization = AuthorizationConfig( + enabled=True, + fail_closed=fail_closed, + default_role=default_role, + ) + + +# ── filter_available_skills_by_authorization ─────────────────────────── + + +def test_filter_available_skills_disabled_is_noop(): + """When authorization is disabled, the allowlist is returned unchanged.""" + app_config = _make_app_config() + assert filter_available_skills_by_authorization({"a", "b"}, context=_context(), app_config=app_config) == {"a", "b"} + assert filter_available_skills_by_authorization(None, context=_context(), app_config=app_config) is None + assert filter_available_skills_by_authorization(set(), context=_context(), app_config=app_config) == set() + + +def test_filter_available_skills_rbac_allow_subset(monkeypatch): + """Role allowlist intersects with the agent allowlist.""" + provider = _rbac_provider({"user": {"skills": {"allow": ["skill-a", "skill-c"]}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b", "skill-c"}, + context=_context(), + app_config=app_config, + ) + assert result == {"skill-a", "skill-c"} + + +def test_filter_available_skills_provider_injected_names_excluded(monkeypatch): + """A buggy provider that returns names outside candidates can't expand the allowlist. + + Regression for zhfeng's P2 finding: filter_resources contract says results + must be a subset of candidates, but only list[str] is validated. The filter + must intersect the provider result with candidates so injected names are dropped. + """ + + class _InjectingProvider: + name = "injecting" + + def filter_resources(self, principal, resource_type, candidates): + # Returns a name not in the candidate set (simulating a buggy provider). + return ["allowed-skill", "injected-skill"] + + def authorize(self, request): + return None + + async def aauthorize(self, request): + return None + + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: _InjectingProvider(), + ) + + result = filter_available_skills_by_authorization( + {"allowed-skill"}, + context=_context(), + app_config=app_config, + ) + # injected-skill was never a candidate, so it must not appear. + assert result == {"allowed-skill"} + assert "injected-skill" not in result + + +def test_filter_available_skills_rbac_deny(monkeypatch): + """Deny removes a skill even when allow is wildcard.""" + provider = _rbac_provider({"user": {"skills": {"allow": "*", "deny": ["skill-b"]}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b", "skill-c"}, + context=_context(), + app_config=app_config, + ) + assert result == {"skill-a", "skill-c"} + + +def test_filter_available_skills_wildcard_returns_all(monkeypatch): + """Allow: '*' preserves the full allowlist.""" + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b"}, + context=_context(), + app_config=app_config, + ) + assert result == {"skill-a", "skill-b"} + + +def test_filter_available_skills_empty_allowlist_preserved(monkeypatch): + """Empty set (no skills for this agent) is preserved as-is.""" + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + result = filter_available_skills_by_authorization( + set(), + context=_context(), + app_config=app_config, + ) + assert result == set() + + +def test_filter_available_skills_provider_error_fail_closed(monkeypatch): + """Provider error + fail_closed → empty set (deny all skills).""" + + class _ErrorProvider: + name = "error" + + def authorize(self, request): + raise RuntimeError("boom") + + async def aauthorize(self, request): + raise RuntimeError("boom") + + def filter_resources(self, principal, resource_type, candidates): + raise RuntimeError("boom") + + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=True) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: _ErrorProvider(), + ) + + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b"}, + context=_context(), + app_config=app_config, + ) + assert result == set() + + +def test_filter_available_skills_provider_error_fail_open(monkeypatch): + """Provider error + fail_open → original allowlist preserved.""" + + class _ErrorProvider: + name = "error" + + def authorize(self, request): + raise RuntimeError("boom") + + async def aauthorize(self, request): + raise RuntimeError("boom") + + def filter_resources(self, principal, resource_type, candidates): + raise RuntimeError("boom") + + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=False) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: _ErrorProvider(), + ) + + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b"}, + context=_context(), + app_config=app_config, + ) + assert result == {"skill-a", "skill-b"} + + +def test_filter_available_skills_internal_caller_uses_default_role(monkeypatch): + """Internal callers (system_role=internal) fall under default_role.""" + provider = _rbac_provider( + { + "user": {"skills": {"allow": ["skill-a"]}}, + "admin": {"skills": {"allow": "*"}}, + } + ) + app_config = _make_app_config() + _enable_authz(app_config, default_role="user") + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + # Internal caller with system_role=None → default_role="user" + result = filter_available_skills_by_authorization( + {"skill-a", "skill-b"}, + context=_context(user_role=None, is_internal=True), + app_config=app_config, + ) + assert result == {"skill-a"} + + +def test_filter_available_skills_candidate_resolution_error_fail_closed(monkeypatch): + """available_skills=None + candidate-set resolution error + fail_closed → empty set. + + Regression for willem-bd's fail-open bypass finding: when there is no + agent-level allowlist (``available_skills=None``) and + ``_all_configured_skill_names`` raises (storage I/O error), the result must + NOT be ``None`` (which would bypass authorization — every skill becomes + activatable/loadable via SkillActivationMiddleware and subagent _load_skills). + With ``fail_closed=true`` it must return an empty set (deny all). + """ + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=True) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr( + "deerflow.authz.skill_filter._all_configured_skill_names", + lambda app_config, **kw: (_ for _ in ()).throw(RuntimeError("storage I/O failed")), + ) + + result = filter_available_skills_by_authorization( + None, + context=_context(), + app_config=app_config, + ) + # fail-closed → deny all skills, NOT None (which would bypass authorization). + assert result == set() + + +def test_filter_available_skills_candidate_resolution_error_fail_open(monkeypatch): + """available_skills=None + candidate-set resolution error + fail_open → None. + + Fail-open preserves the legacy "no constraint" behavior (None = unrestricted), + matching how provider errors are handled in the fail-open branch above. + """ + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=False) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr( + "deerflow.authz.skill_filter._all_configured_skill_names", + lambda app_config, **kw: (_ for _ in ()).throw(RuntimeError("storage I/O failed")), + ) + + result = filter_available_skills_by_authorization( + None, + context=_context(), + app_config=app_config, + ) + # fail-open → unrestricted (None), matching provider-error fail-open behavior. + assert result is None + + +def test_filter_available_skills_none_with_empty_config_no_bypass(monkeypatch): + """available_skills=None + zero configured skills → None (no bypass, nothing to load). + + Distinct from the resolution-error case: when ``_all_configured_skill_names`` + succeeds and returns an empty list (the user genuinely configured no skills), + returning ``None`` is safe because there is nothing for SkillActivationMiddleware + or subagent _load_skills to activate/load. + """ + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=True) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + # Resolution succeeds, but the configured-skill set is genuinely empty. + monkeypatch.setattr( + "deerflow.authz.skill_filter._all_configured_skill_names", + lambda app_config, **kw: [], + ) + + result = filter_available_skills_by_authorization( + None, + context=_context(), + app_config=app_config, + ) + assert result is None + + +def test_filter_available_skills_user_id_threads_into_candidate_resolution(monkeypatch): + """user_id is forwarded to _all_configured_skill_names so per-user custom skills are candidates. + + Regression for willem-bd's Round 3 finding: when available_skills=None and + authz is enabled, the candidate universe must include per-user custom skills + (UserScopedSkillStorage), not just the process-global public skills. Otherwise + a skills:{allow:"*"} wildcard can never reach per-user custom skills, and + SkillActivationMiddleware blocks them. + """ + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + captured = {} + + def _record(app_config, *, user_id=None): + captured["user_id"] = user_id + # Simulate a per-user custom skill that only user-scoped storage would return. + return ["public-skill", "user-custom-skill"] + + monkeypatch.setattr( + "deerflow.authz.skill_filter._all_configured_skill_names", + _record, + ) + + result = filter_available_skills_by_authorization( + None, + context=_context(user_id="user-123"), + app_config=app_config, + user_id="user-123", + ) + # The user_id was forwarded to the resolver. + assert captured["user_id"] == "user-123" + # The per-user custom skill is in the filtered set (wildcard allows all). + assert result == {"public-skill", "user-custom-skill"} + + +def test_filter_available_skills_omits_user_id_when_not_provided(monkeypatch): + """When user_id is not passed, candidate resolution uses global storage (user_id=None).""" + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + + captured = {} + + def _record(app_config, *, user_id=None): + captured["user_id"] = user_id + return ["public-skill"] + + monkeypatch.setattr( + "deerflow.authz.skill_filter._all_configured_skill_names", + _record, + ) + + filter_available_skills_by_authorization( + None, + context=_context(), + app_config=app_config, + # user_id deliberately omitted (e.g. caller has no user context). + ) + assert captured["user_id"] is None + + +# ── filter_resources_by_authorization (generic) ──────────────────────── + + +def _named(name: str): + return SimpleNamespace(name=name) + + +def test_filter_resources_by_authorization_no_provider(): + """No provider → original list returned.""" + principal = build_principal_from_context(_context(), default_role="user") + resources = [_named("a"), _named("b")] + assert filter_resources_by_authorization(resources, resource_type="skill", provider=None, principal=principal, fail_closed=True) == resources + + +def test_filter_resources_by_authorization_rbac(monkeypatch): + """Generic filter works with any resource type.""" + provider = _rbac_provider({"user": {"skills": {"allow": ["a"], "deny": ["c"]}}}) + principal = build_principal_from_context(_context(), default_role="user") + resources = [_named("a"), _named("b"), _named("c")] + + result = filter_resources_by_authorization( + resources, + resource_type="skill", + provider=provider, + principal=principal, + fail_closed=True, + ) + assert [r.name for r in result] == ["a"] + + +def test_filter_resources_by_authorization_provider_error_fail_closed(): + """Provider error + fail_closed → empty list.""" + + class _ErrorProvider: + name = "error" + + def authorize(self, request): + raise RuntimeError("boom") + + async def aauthorize(self, request): + raise RuntimeError("boom") + + def filter_resources(self, principal, resource_type, candidates): + raise RuntimeError("boom") + + principal = build_principal_from_context(_context(), default_role="user") + resources = [_named("a"), _named("b")] + result = filter_resources_by_authorization( + resources, + resource_type="skill", + provider=_ErrorProvider(), + principal=principal, + fail_closed=True, + ) + assert result == [] + + +def test_filter_resources_by_authorization_preserves_order(): + """Original order is preserved in the filtered result.""" + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + principal = build_principal_from_context(_context(), default_role="user") + resources = [_named("z"), _named("a"), _named("m")] + result = filter_resources_by_authorization( + resources, + resource_type="skill", + provider=provider, + principal=principal, + fail_closed=True, + ) + assert [r.name for r in result] == ["z", "a", "m"] + + +# ── DeerFlowClient._ensure_agent path ───────────────────────────────── +# Regression for willem-bd's Round 2 coverage observation: the embedded +# lead-agent construction path (DeerFlowClient._ensure_agent) must filter +# skills through authorization too, mirroring _make_lead_agent. Otherwise a +# caller building the agent via DeerFlowClient(available_skills=...) bypasses +# the role's skills policy. + + +def test_client_ensure_agent_filters_skills_by_authorization(monkeypatch): + """_ensure_agent passes an authorization-filtered skill set to build_middlewares. + + Real-path test: a genuine RbacAuthorizationProvider denies 'denied-skill' + for the 'user' role. The embedded _ensure_agent must filter it out before + wiring SkillActivationMiddleware, so the denied skill can't be slash-activated. + """ + from langchain_core.runnables import RunnableConfig + + from deerflow.client import DeerFlowClient + + app_config = _make_app_config() + _enable_authz(app_config) + provider = _rbac_provider({"user": {"skills": {"allow": ["allowed-skill"], "deny": ["denied-skill"]}}}) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr( + "deerflow.authz.tool_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"tools": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.agents.lead_agent.agent.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"models": {"allow": "*"}}}), + ) + + captured = {} + + def _capture_build_middlewares(*args, **kwargs): + captured["available_skills"] = kwargs.get("available_skills") + return [] + + monkeypatch.setattr("deerflow.client.create_chat_model", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.create_agent", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.build_middlewares", _capture_build_middlewares) + monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", lambda app_config, **kw: []) + monkeypatch.setattr( + "deerflow.client.build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), + ) + monkeypatch.setattr( + "deerflow.client.assemble_deferred_tools", + lambda tools, *, enabled: ([], SimpleNamespace(deferred_names=frozenset())), + ) + monkeypatch.setattr("deerflow.client.build_mcp_routing_middleware", lambda *a, **kw: None) + monkeypatch.setattr("deerflow.client.get_mcp_routing_hints_prompt_section", lambda *a, **kw: "") + monkeypatch.setattr("deerflow.client.apply_prompt_template", lambda **kw: "") + monkeypatch.setattr("deerflow.client.get_thread_state_schema", lambda *a, **kw: object()) + monkeypatch.setattr("deerflow.client.normalize_middleware_state_schemas", lambda schemas, mode, freq: []) + monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") + + client = DeerFlowClient.__new__(DeerFlowClient) + client._app_config = app_config + client._agent_name = "default" + client._available_skills = {"allowed-skill", "denied-skill"} + client._checkpoint_channel_mode = "full" + client._checkpoint_snapshot_frequency = None + client._middlewares = [] + client._agent = None + client._agent_config_key = None + client._checkpointer = object() + + config = RunnableConfig(configurable={"user_id": "user-123", "user_role": "user"}) + client._ensure_agent(config) + + # The denied skill was filtered out; only 'allowed-skill' reaches the middleware. + assert captured["available_skills"] == {"allowed-skill"} + + +def test_client_ensure_agent_noop_when_authorization_disabled(monkeypatch): + """When authorization is disabled, _ensure_agent leaves the skill set unchanged.""" + from langchain_core.runnables import RunnableConfig + + from deerflow.client import DeerFlowClient + + app_config = _make_app_config() + # AuthorizationConfig() defaults to enabled=False. + + captured = {} + + def _capture_build_middlewares(*args, **kwargs): + captured["available_skills"] = kwargs.get("available_skills") + return [] + + monkeypatch.setattr("deerflow.client.create_chat_model", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.create_agent", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.build_middlewares", _capture_build_middlewares) + monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", lambda app_config, **kw: []) + monkeypatch.setattr( + "deerflow.client.build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), + ) + monkeypatch.setattr( + "deerflow.client.assemble_deferred_tools", + lambda tools, *, enabled: ([], SimpleNamespace(deferred_names=frozenset())), + ) + monkeypatch.setattr("deerflow.client.build_mcp_routing_middleware", lambda *a, **kw: None) + monkeypatch.setattr("deerflow.client.get_mcp_routing_hints_prompt_section", lambda *a, **kw: "") + monkeypatch.setattr("deerflow.client.apply_prompt_template", lambda **kw: "") + monkeypatch.setattr("deerflow.client.get_thread_state_schema", lambda *a, **kw: object()) + monkeypatch.setattr("deerflow.client.normalize_middleware_state_schemas", lambda schemas, mode, freq: []) + monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") + + client = DeerFlowClient.__new__(DeerFlowClient) + client._app_config = app_config + client._agent_name = "default" + client._available_skills = {"skill-a", "skill-b"} + client._checkpoint_channel_mode = "full" + client._checkpoint_snapshot_frequency = None + client._middlewares = [] + client._agent = None + client._agent_config_key = None + client._checkpointer = object() + + config = RunnableConfig(configurable={"user_id": "user-123", "user_role": "user"}) + client._ensure_agent(config) + + # Disabled → no-op: original skill set passed through unchanged. + assert captured["available_skills"] == {"skill-a", "skill-b"} + + +# ── Runtime skill:activate enforcement (Layer 2) ────────────────────── + + +class _ActionAwareProvider: + """Minimal provider distinguishing visibility (filter_resources) from activation.""" + + def __init__(self, *, denied_activate: set[str]): + self.denied_activate = denied_activate + self.sync_calls: list[str] = [] + self.async_calls: list[str] = [] + + def filter_resources(self, principal, resource, candidates): + return list(candidates) + + def authorize(self, request): + from deerflow.authz.provider import AuthzDecision + + if request.resource == "skill" and request.action == "activate": + self.sync_calls.append(request.target) + if request.resource == "skill" and request.action == "activate" and request.target in self.denied_activate: + return AuthzDecision(allow=False) + return AuthzDecision(allow=True) + + async def aauthorize(self, request): + from deerflow.authz.provider import AuthzDecision + + if request.resource == "skill" and request.action == "activate": + self.async_calls.append(request.target) + if request.resource == "skill" and request.action == "activate" and request.target in self.denied_activate: + return AuthzDecision(allow=False) + return AuthzDecision(allow=True) + + +class _RaisingAuthorizeProvider(_ActionAwareProvider): + def authorize(self, request): + raise RuntimeError("provider blew up") + + async def aauthorize(self, request): + raise RuntimeError("provider blew up") + + +class _AsyncOnlyProvider: + """Loop-affine provider: the sync API is the wrong one and always fails. + + Models a provider whose clients are bound to the running event loop — + ``authorize()`` from a worker thread or the loop itself is unsupported, + while ``aauthorize()`` works. Async paths must use the async API; if they + fall back to the sync call, fail-closed denies (or fail-open admits) with + the wrong semantics. + """ + + name = "async-only" + + def __init__(self, *, denied_activate: set[str] = set()): + self.denied_activate = denied_activate + self.async_calls: list[str] = [] + + def filter_resources(self, principal, resource, candidates): + return list(candidates) + + def authorize(self, request): + raise RuntimeError("sync authorize() is not supported by this provider") + + async def aauthorize(self, request): + from deerflow.authz.provider import AuthzDecision + + if request.resource == "skill" and request.action == "activate": + self.async_calls.append(request.target) + if request.resource == "skill" and request.action == "activate" and request.target in self.denied_activate: + return AuthzDecision(allow=False) + return AuthzDecision(allow=True) + + +def _resolved_skill_authorization(provider, *, fail_closed: bool): + from deerflow.authz.skill_filter import resolve_skill_authorization + + app_config = _make_app_config() + _enable_authz(app_config, fail_closed=fail_closed) + import deerflow.authz.skill_filter as skill_filter_module + + original = skill_filter_module.resolve_authorization_provider + skill_filter_module.resolve_authorization_provider = lambda config: provider + try: + resolved = resolve_skill_authorization(_context(), app_config) + finally: + skill_filter_module.resolve_authorization_provider = original + assert resolved is not None + return resolved + + +def _middleware_for(tmp_path, monkeypatch, skills, **kwargs): + from pathlib import Path + + from deerflow.agents.middlewares import skill_activation_middleware as middleware_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + def _make_skill(name: str) -> SkillObject: + skill_dir = tmp_path / name + skill_dir.mkdir(exist_ok=True) + skill_file = skill_dir / "SKILL.md" + skill_file.write_text(f"# {name}\nskill body", encoding="utf-8") + return SkillObject( + name=name, + description=f"Description for {name}", + license="MIT", + skill_dir=skill_dir, + skill_file=skill_file, + relative_path=Path(name), + category=SkillCategory.CUSTOM, + enabled=True, + ) + + made = [_make_skill(name) for name in skills] + + def _validate_skill_file_path(skill_file): + resolved = skill_file.resolve() + resolved.relative_to(tmp_path.resolve()) + return resolved + + storage = SimpleNamespace( + load_skills=lambda *, enabled_only: made, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + validate_skill_file_path=_validate_skill_file_path, + ) + monkeypatch.setattr(middleware_module, "get_or_new_skill_storage", lambda **kw: storage) + return SkillActivationMiddleware(slash_source_owner_token="test-token", **kwargs) + + +def test_activation_enforces_action_scoped_activate_decision(tmp_path, monkeypatch): + """A skill visible in the Layer 1 set but denied by authorize(activate) + must not slash-activate, while an allowed one still does (review repro).""" + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill", "other-skill"], + available_skills={"demo-skill", "other-skill"}, + skill_authorization=resolved, + ) + + denied = middleware._resolve_activation("/demo-skill analyze this") + assert denied is not None + assert denied.activation is None + assert "not available" in denied.failure_message + + allowed = middleware._resolve_activation("/other-skill analyze this") + assert allowed is not None + assert allowed.activation is not None + assert allowed.activation.skill_name == "other-skill" + + +def test_activation_provider_error_fail_closed(tmp_path, monkeypatch): + """authorize() raising under fail_closed=True denies activation.""" + provider = _RaisingAuthorizeProvider(denied_activate=set()) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill"], + available_skills={"demo-skill"}, + skill_authorization=resolved, + ) + + result = middleware._resolve_activation("/demo-skill hi") + assert result is not None + assert result.activation is None + assert "not available" in result.failure_message + + +def test_activation_provider_error_fail_open(tmp_path, monkeypatch): + """authorize() raising under fail_closed=False allows (fail-open policy).""" + provider = _RaisingAuthorizeProvider(denied_activate=set()) + resolved = _resolved_skill_authorization(provider, fail_closed=False) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill"], + available_skills={"demo-skill"}, + skill_authorization=resolved, + ) + + result = middleware._resolve_activation("/demo-skill hi") + assert result is not None + assert result.activation is not None + + +def test_activation_rbac_provider_membership_equivalent(tmp_path, monkeypatch): + """Built-in RBAC (action-agnostic) keeps working: allowed skill activates.""" + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill"], + available_skills={"demo-skill"}, + skill_authorization=resolved, + ) + + result = middleware._resolve_activation("/demo-skill hi") + assert result is not None + assert result.activation is not None + + +def test_client_wires_skill_authorization_into_middleware(monkeypatch): + """_ensure_agent resolves the skill authorization context once and passes + it to build_middlewares so SkillActivationMiddleware enforces skill:activate.""" + from langchain_core.runnables import RunnableConfig + + from deerflow.authz.skill_filter import ResolvedSkillAuthorization + from deerflow.client import DeerFlowClient + + app_config = _make_app_config() + _enable_authz(app_config) + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr( + "deerflow.authz.tool_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"tools": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.agents.lead_agent.agent.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"models": {"allow": "*"}}}), + ) + + captured = {} + + def _capture_build_middlewares(*args, **kwargs): + captured["skill_authorization"] = kwargs.get("skill_authorization") + return [] + + monkeypatch.setattr("deerflow.client.create_chat_model", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.create_agent", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.build_middlewares", _capture_build_middlewares) + monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", lambda app_config, **kw: []) + monkeypatch.setattr( + "deerflow.client.build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), + ) + monkeypatch.setattr( + "deerflow.client.assemble_deferred_tools", + lambda tools, *, enabled: ([], SimpleNamespace(deferred_names=frozenset())), + ) + monkeypatch.setattr("deerflow.client.build_mcp_routing_middleware", lambda *a, **kw: None) + monkeypatch.setattr("deerflow.client.get_mcp_routing_hints_prompt_section", lambda *a, **kw: "") + monkeypatch.setattr("deerflow.client.apply_prompt_template", lambda **kw: "") + monkeypatch.setattr("deerflow.client.get_thread_state_schema", lambda *a, **kw: object()) + monkeypatch.setattr("deerflow.client.normalize_middleware_state_schemas", lambda schemas, mode, freq: []) + monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") + + client = DeerFlowClient.__new__(DeerFlowClient) + client._app_config = app_config + client._agent_name = "default" + client._available_skills = None + client._checkpoint_channel_mode = "full" + client._checkpoint_snapshot_frequency = None + client._middlewares = [] + client._agent = None + client._agent_config_key = None + client._checkpointer = object() + + client._ensure_agent(RunnableConfig(configurable={"user_id": "user-123", "user_role": "user"})) + + wired = captured["skill_authorization"] + assert isinstance(wired, ResolvedSkillAuthorization) + assert wired.provider is provider + assert wired.fail_closed is True + + +def test_client_filter_candidates_reuse_catalog_loader(monkeypatch): + """With no agent-level allowlist, the filter's candidates come from the + cached catalog loader — the filter's uncached storage scan never runs.""" + from langchain_core.runnables import RunnableConfig + + from deerflow.client import DeerFlowClient + + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"skills": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.authz.tool_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"tools": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.agents.lead_agent.agent.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"models": {"allow": "*"}}}), + ) + + catalog_calls = {"count": 0} + + def _catalog_loader(app_config, **kw): + catalog_calls["count"] += 1 + return [SimpleNamespace(name="catalog-skill")] + + def _must_not_scan(*a, **kw): + raise AssertionError("filter must not rescan storage when candidates were provided") + + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", _catalog_loader) + monkeypatch.setattr("deerflow.authz.skill_filter._all_configured_skill_names", _must_not_scan) + + import deerflow.authz.skill_filter as skill_filter_module + + captured = {} + _original_filter = skill_filter_module.filter_available_skills_by_authorization + + def _filter_spy(available_skills, **kwargs): + captured["candidate_skill_names"] = kwargs.get("candidate_skill_names") + return _original_filter(available_skills, **kwargs) + + # client._ensure_agent lazy-imports the filter from this module, so the + # spy has to replace it at the source. + monkeypatch.setattr(skill_filter_module, "filter_available_skills_by_authorization", _filter_spy) + monkeypatch.setattr("deerflow.client.create_chat_model", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.create_agent", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.build_middlewares", lambda *a, **kw: []) + monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 + monkeypatch.setattr( + "deerflow.client.build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), + ) + monkeypatch.setattr( + "deerflow.client.assemble_deferred_tools", + lambda tools, *, enabled: ([], SimpleNamespace(deferred_names=frozenset())), + ) + monkeypatch.setattr("deerflow.client.build_mcp_routing_middleware", lambda *a, **kw: None) + monkeypatch.setattr("deerflow.client.get_mcp_routing_hints_prompt_section", lambda *a, **kw: "") + monkeypatch.setattr("deerflow.client.apply_prompt_template", lambda **kw: "") + monkeypatch.setattr("deerflow.client.get_thread_state_schema", lambda *a, **kw: object()) + monkeypatch.setattr("deerflow.client.normalize_middleware_state_schemas", lambda schemas, mode, freq: []) + monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") + + client = DeerFlowClient.__new__(DeerFlowClient) + client._app_config = app_config + client._agent_name = "default" + client._available_skills = None # no agent-level allowlist → candidates matter + client._checkpoint_channel_mode = "full" + client._checkpoint_snapshot_frequency = None + client._middlewares = [] + client._agent = None + client._agent_config_key = None + client._checkpointer = object() + + client._ensure_agent(RunnableConfig(configurable={"user_id": "user-123", "user_role": "user"})) + + assert captured["candidate_skill_names"] == ["catalog-skill"] + assert catalog_calls["count"] >= 1 + + +def test_subagent_chain_arms_skill_activate_check(monkeypatch): + """build_subagent_runtime_middlewares forwards skill_authorization into the + subagent chain's SkillActivationMiddleware (a delegated task is a plain + HumanMessage, so /skill-name in task text reaches the activation path).""" + import deerflow.authz.skill_filter as skill_filter_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.agents.middlewares.tool_error_handling_middleware import build_subagent_runtime_middlewares + from deerflow.authz.skill_filter import resolve_skill_authorization + + app_config = _make_app_config() + _enable_authz(app_config) + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + monkeypatch.setattr(skill_filter_module, "resolve_authorization_provider", lambda config: provider) + + resolved = resolve_skill_authorization(_context(), app_config) + assert resolved is not None + + middlewares = build_subagent_runtime_middlewares( + app_config=app_config, + model_name=None, + lazy_init=True, + available_skills={"demo-skill"}, + user_id="user-123", + authorization_provider=provider, + skill_authorization=resolved, + ) + + activation = [m for m in middlewares if isinstance(m, SkillActivationMiddleware)] + assert len(activation) == 1 + assert activation[0]._skill_authorization is resolved + + # The skill-read stamp gate on the same chain shares the resolved context + # so an autonomously read SKILL.md cannot activate a denied skill. + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + stampers = [m for m in middlewares if isinstance(m, ToolErrorHandlingMiddleware)] + assert len(stampers) == 1 + assert stampers[0]._skill_authorization is resolved + + +def test_subagent_executor_resolves_skill_authorization_for_chain(monkeypatch): + """SubagentExecutor._create_agent passes a resolved ResolvedSkillAuthorization + into build_subagent_runtime_middlewares, built from the executor identity.""" + import importlib + import sys + from types import SimpleNamespace + + import deerflow.authz.skill_filter as skill_filter_module + from deerflow.authz.skill_filter import ResolvedSkillAuthorization + + # tests/conftest.py injects a MagicMock for deerflow.subagents.executor to + # break a production circular import; load the real module for this test + # (same pattern as tests/test_delegation_ledger_live.py). + sys.modules.pop("deerflow.subagents.executor", None) + executor_module = importlib.import_module("deerflow.subagents.executor") + SubagentExecutor = executor_module.SubagentExecutor + + app_config = _make_app_config() + _enable_authz(app_config) + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + monkeypatch.setattr(skill_filter_module, "resolve_authorization_provider", lambda config: provider) + monkeypatch.setattr(executor_module, "create_chat_model", lambda **kw: object()) + monkeypatch.setattr(executor_module, "resolve_subagent_model_name", lambda *a, **kw: "m") + + captured = {} + + def _capture_builder(**kwargs): + captured["skill_authorization"] = kwargs.get("skill_authorization") + return [] + + monkeypatch.setattr( + "deerflow.agents.middlewares.tool_error_handling_middleware.build_subagent_runtime_middlewares", + _capture_builder, + ) + + executor = SubagentExecutor.__new__(SubagentExecutor) + executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25) + executor.model_name = "m" + executor.app_config = app_config + executor._resolved_app_config = app_config + executor._available_skill_names = None + executor.extensions = None + executor.user_id = "user-123" + executor.trace_id = "test-trace" + executor.user_role = "user" + executor.oauth_provider = None + executor.oauth_id = None + executor.channel_user_id = None + executor.is_internal = False + executor.authz_attributes = None + executor.tools = [] + + executor._create_agent(tools=[]) + + wired = captured["skill_authorization"] + assert isinstance(wired, ResolvedSkillAuthorization) + assert wired.provider is provider + + +def test_client_skill_surface_uses_one_effective_user(monkeypatch): + """The filter, the candidate pre-load, and the catalog all resolve the same + effective user — a request whose configurable carries no user_id must not + fall back to the process-global skill bucket while the middleware loads + user-scoped storage (per-user custom skills would then be filtered out).""" + from langchain_core.runnables import RunnableConfig + + from deerflow.client import DeerFlowClient + + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.skill_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"skills": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.authz.tool_filter.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"tools": {"allow": "*"}}}), + ) + monkeypatch.setattr( + "deerflow.agents.lead_agent.agent.resolve_authorization_provider", + lambda config: _rbac_provider({"user": {"models": {"allow": "*"}}}), + ) + + catalog_calls = [] + + def _catalog_loader(app_config, **kw): + catalog_calls.append(kw.get("user_id")) + return [SimpleNamespace(name="catalog-skill")] + + monkeypatch.setattr("deerflow.client.get_enabled_skills_for_config", _catalog_loader) + + import deerflow.authz.skill_filter as skill_filter_module + + captured = {} + _original_filter = skill_filter_module.filter_available_skills_by_authorization + + def _filter_spy(available_skills, **kwargs): + captured["user_id"] = kwargs.get("user_id") + return _original_filter(available_skills, **kwargs) + + monkeypatch.setattr(skill_filter_module, "filter_available_skills_by_authorization", _filter_spy) + monkeypatch.setattr("deerflow.client.create_chat_model", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.create_agent", lambda **kw: object()) + monkeypatch.setattr("deerflow.client.build_middlewares", lambda *a, **kw: []) + monkeypatch.setattr("deerflow.client.DeerFlowClient._get_tools", staticmethod(lambda *, model_name, subagent_enabled, mcp_plugins=None: [])) # noqa: ARG005 + monkeypatch.setattr( + "deerflow.client.build_skill_search_setup", + lambda skills, *, enabled, container_base_path, skill_authorization=None: SimpleNamespace(describe_skill_tool=None, skill_names=frozenset()), + ) + monkeypatch.setattr( + "deerflow.client.assemble_deferred_tools", + lambda tools, *, enabled: ([], SimpleNamespace(deferred_names=frozenset())), + ) + monkeypatch.setattr("deerflow.client.build_mcp_routing_middleware", lambda *a, **kw: None) + monkeypatch.setattr("deerflow.client.get_mcp_routing_hints_prompt_section", lambda *a, **kw: "") + monkeypatch.setattr("deerflow.client.apply_prompt_template", lambda **kw: "") + monkeypatch.setattr("deerflow.client.get_thread_state_schema", lambda *a, **kw: object()) + monkeypatch.setattr("deerflow.client.normalize_middleware_state_schemas", lambda schemas, mode, freq: []) + # No user_id in configurable: the effective id must come from here. + monkeypatch.setattr("deerflow.client.get_effective_user_id", lambda: "user-123") + + client = DeerFlowClient.__new__(DeerFlowClient) + client._app_config = app_config + client._agent_name = "default" + client._available_skills = None + client._checkpoint_channel_mode = "full" + client._checkpoint_snapshot_frequency = None + client._middlewares = [] + client._agent = None + client._agent_config_key = None + client._checkpointer = object() + + client._ensure_agent(RunnableConfig(configurable={"user_role": "user"})) + + # The filter, the candidate pre-load, and the catalog all used the + # context-scoped effective user, never the global bucket. + assert captured["user_id"] == "user-123" + assert catalog_calls and all(call == "user-123" for call in catalog_calls) + + +# ── Autonomous path: describe_skill + skill-file-load gates (Layer 2) ── + + +def _describe_setup_for(provider, *, fail_closed: bool = True, skills=("demo-skill", "other-skill")): + """Build a real describe_skill tool over *skills* with the provider wired.""" + from pathlib import Path + + from deerflow.skills.catalog import SkillCatalog + from deerflow.skills.describe import build_describe_skill_tool + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + made = [] + for name in skills: + base = Path("/mnt/skills/public") / name + made.append( + SkillObject( + name=name, + description=f"Description for {name}", + license=None, + skill_dir=base, + skill_file=base / "SKILL.md", + relative_path=Path(name), + category=SkillCategory.PUBLIC, + enabled=True, + ) + ) + resolved = _resolved_skill_authorization(provider, fail_closed=fail_closed) + return build_describe_skill_tool(SkillCatalog(tuple(made)), skill_authorization=resolved) + + +def _invoke_describe(tool, query: str) -> str: + result = tool.invoke({"args": {"name": query}, "name": "describe_skill", "type": "tool_call", "id": "describe-call"}) + return result.update["messages"][0].content + + +def test_describe_skill_gates_on_activate_decision(): + """describe_skill omits skills denied by authorize(skill, activate) even + though the Layer 1 catalog lists them — the autonomous load path must not + be able to discover and read_file a skill whose activation is denied.""" + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + tool = _describe_setup_for(provider) + + content = _invoke_describe(tool, "select:demo-skill,other-skill") + + assert "other-skill" in content + assert "demo-skill" not in content + + +def test_describe_skill_denied_only_reports_no_match(): + """A describe whose every match is denied is indistinguishable from a + non-match (the denial reason is not leaked).""" + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + tool = _describe_setup_for(provider, skills=("demo-skill",)) + + content = _invoke_describe(tool, "select:demo-skill") + + assert "No skills matched" in content + + +def test_describe_skill_provider_error_fail_closed(): + provider = _RaisingAuthorizeProvider(denied_activate=set()) + tool = _describe_setup_for(provider, fail_closed=True) + + content = _invoke_describe(tool, "select:demo-skill") + + assert "No skills matched" in content + + +def test_describe_skill_provider_error_fail_open(): + provider = _RaisingAuthorizeProvider(denied_activate=set()) + tool = _describe_setup_for(provider, fail_closed=False) + + content = _invoke_describe(tool, "select:demo-skill") + + assert "demo-skill" in content + + +def test_describe_skill_disabled_authorization_describes_all(): + """Without a resolved authorization context the tool describes everything + in the catalog (pre-authorization behavior).""" + from pathlib import Path + + from deerflow.skills.catalog import SkillCatalog + from deerflow.skills.describe import build_describe_skill_tool + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + made = [] + for name in ("demo-skill", "other-skill"): + base = Path("/mnt/skills/public") / name + made.append( + SkillObject( + name=name, + description=f"Description for {name}", + license=None, + skill_dir=base, + skill_file=base / "SKILL.md", + relative_path=Path(name), + category=SkillCategory.PUBLIC, + enabled=True, + ) + ) + tool = build_describe_skill_tool(SkillCatalog(tuple(made))) + + content = _invoke_describe(tool, "select:demo-skill,other-skill") + + assert "demo-skill" in content + assert "other-skill" in content + + +# ── Skill-file-load stamping gate ───────────────────────────────────── + + +def _read_call_and_message(path: str): + from langchain_core.messages import ToolMessage + + request = SimpleNamespace(tool_call={"name": "read_file", "id": "call-1", "args": {"path": path}}) + message = ToolMessage(content="---\ndescription: Demo skill\n---\n# demo", tool_call_id="call-1", name="read_file") + return request, message + + +def test_skill_read_stamp_gates_on_activate_decision(): + """A completed SKILL.md read only records a skill_context entry when the + action-scoped skill:activate decision allows it; a denied read gets the + denial marker instead (no durable context, tool policy, or secrets).""" + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + + request, message = _read_call_and_message("/mnt/skills/public/demo-skill/SKILL.md") + stamped = middleware._stamp_skill_read_metadata(message, request, tool_name="read_file") + assert SKILL_CONTEXT_ENTRY_KEY not in stamped.additional_kwargs + assert stamped.additional_kwargs.get(SKILL_CONTEXT_DENIED_KEY) is True + + request_ok, message_ok = _read_call_and_message("/mnt/skills/public/other-skill/SKILL.md") + stamped_ok = middleware._stamp_skill_read_metadata(message_ok, request_ok, tool_name="read_file") + assert SKILL_CONTEXT_DENIED_KEY not in stamped_ok.additional_kwargs + assert SKILL_CONTEXT_ENTRY_KEY in stamped_ok.additional_kwargs + + +def test_extract_skills_skips_denied_reads_without_warning(caplog): + """extract_skills treats the denial marker as intentional — no entry and + no misleading 'missing skill read metadata' warning.""" + from langchain_core.messages import AIMessage, ToolMessage + + from deerflow.agents.middlewares import skill_context as skill_context_module + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY + + messages = [ + AIMessage(content="", tool_calls=[{"name": "read_file", "id": "call-1", "args": {"path": "/mnt/skills/public/demo-skill/SKILL.md"}}]), + ToolMessage(content="# demo", tool_call_id="call-1", name="read_file", additional_kwargs={SKILL_CONTEXT_DENIED_KEY: True}), + ] + + with caplog.at_level("WARNING", logger="deerflow.agents.middlewares.skill_context"): + entries = skill_context_module.extract_skills(messages, skills_root="/mnt/skills", read_tool_names={"read_file"}) + + assert entries == [] + assert not caplog.records + + +# ── Runtime-chain wiring for the autonomous-path gates ──────────────── + + +def test_lead_runtime_chain_forwards_skill_authorization_to_stamp_gate(): + """build_lead_runtime_middlewares hands the resolved skill authorization to + ToolErrorHandlingMiddleware so the skill-read stamp gate runs with the same + provider instance the activation middleware uses — and the effective user + id, so the stamp gate canonicalizes read paths against the same + user-scoped registry the activation/policy middlewares resolve (a global + registry would fall back to path-derived names for per-user custom + skills, producing a wrong authorization target).""" + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware, build_lead_runtime_middlewares + + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + + middlewares = build_lead_runtime_middlewares(app_config=_make_app_config(), skill_authorization=resolved, user_id="user-123") + + stampers = [m for m in middlewares if isinstance(m, ToolErrorHandlingMiddleware)] + assert len(stampers) == 1 + assert stampers[0]._skill_authorization is resolved + assert stampers[0]._user_id == "user-123" + + +def test_subagent_runtime_chain_forwards_user_id_to_stamp_gate(): + """build_subagent_runtime_middlewares hands the effective user id to + ToolErrorHandlingMiddleware: the subagent stamp gate must canonicalize + skill-read paths against the user-scoped registry, matching the + activation middleware constructed in the same builder.""" + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware, build_subagent_runtime_middlewares + + middlewares = build_subagent_runtime_middlewares(app_config=_make_app_config(), user_id="user-456") + + stampers = [m for m in middlewares if isinstance(m, ToolErrorHandlingMiddleware)] + assert len(stampers) == 1 + assert stampers[0]._user_id == "user-456" + + +def test_subagent_executor_shares_one_skill_authorization_instance(monkeypatch): + """[P2 regression] A provider factory returning a distinct instance per + resolve must not split the subagent's layers: the Layer 1 filter in + ``_load_skills`` and the runtime ``skill:activate`` checks in + ``_create_agent`` see the same ResolvedSkillAuthorization (one resolve per + executor assembly).""" + import asyncio + import importlib + import sys + + import deerflow.authz.skill_filter as skill_filter_module + from deerflow.authz.skill_filter import ResolvedSkillAuthorization + + sys.modules.pop("deerflow.subagents.executor", None) + executor_module = importlib.import_module("deerflow.subagents.executor") + SubagentExecutor = executor_module.SubagentExecutor + + resolved_providers: list = [] + + def _distinct_factory(config): + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + resolved_providers.append(provider) + return provider + + monkeypatch.setattr(skill_filter_module, "resolve_authorization_provider", _distinct_factory) + + filter_captured: dict = {} + _original_filter = skill_filter_module.filter_available_skills_by_authorization + + def _filter_spy(available_skills, **kwargs): + filter_captured["authorization"] = kwargs.get("authorization") + return _original_filter(available_skills, **kwargs) + + monkeypatch.setattr(skill_filter_module, "filter_available_skills_by_authorization", _filter_spy) + + app_config = _make_app_config() + _enable_authz(app_config) + + monkeypatch.setattr( + "deerflow.skills.storage.get_or_new_user_skill_storage", + lambda user_id, **kw: SimpleNamespace(load_skills=lambda *, enabled_only: [SimpleNamespace(name="demo-skill")]), + ) + + middleware_captured: dict = {} + + def _capture_builder(**kwargs): + middleware_captured["skill_authorization"] = kwargs.get("skill_authorization") + return [] + + monkeypatch.setattr( + "deerflow.agents.middlewares.tool_error_handling_middleware.build_subagent_runtime_middlewares", + _capture_builder, + ) + monkeypatch.setattr(executor_module, "create_chat_model", lambda **kw: object()) + + executor = SubagentExecutor.__new__(SubagentExecutor) + executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25) + executor.model_name = "m" + executor.app_config = app_config + executor._resolved_app_config = app_config + executor._available_skill_names = None + executor._skill_authorization = None + executor._stop_reason_middlewares = [] + executor.extensions = None + executor.trace_id = "test-trace" + executor.user_id = "user-123" + executor.trace_id = "test-trace" + executor.user_role = "user" + executor.oauth_provider = None + executor.oauth_id = None + executor.channel_user_id = None + executor.is_internal = False + executor.authz_attributes = None + executor.tools = [] + + asyncio.run(executor._load_skills()) + executor._create_agent(tools=[]) + + layer1 = filter_captured["authorization"] + layer2 = middleware_captured["skill_authorization"] + assert isinstance(layer1, ResolvedSkillAuthorization) + assert layer2 is layer1 + # One resolve per assembly even though the factory returns distinct objects. + assert len(resolved_providers) == 1 + + +# ── Re-authorization of persisted skill_context entries (review round 5) ── + + +def test_in_context_secret_sources_reauthorize_persisted_entries(tmp_path, monkeypatch): + """[P1 regression] An entry stamped while activation was allowed must stop + binding secrets on the next run once the provider denies skill:activate — + skill_context persists across runs; the decision does not.""" + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill"], + available_skills={"demo-skill"}, + skill_authorization=resolved, + ) + + def _make_skill_with_secrets(name: str): + made = middleware._storage().load_skills(enabled_only=True) + skill = next(s for s in made if s.name == name) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name="API_KEY", optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + return skill + + skill = _make_skill_with_secrets("demo-skill") + container_root = middleware._storage().get_container_root() + registry = {posixpath_normpath(skill.get_container_file_path(container_root)): skill} + request = SimpleNamespace(state={"skill_context": [{"name": "demo-skill", "path": skill.get_container_file_path(container_root)}]}) + + # Run 1: activation allowed -> the persisted entry binds its secrets. + provider.denied_activate = set() + sources = middleware._in_context_secret_sources(request, registry) + assert [name for name, _ in sources] == ["demo-skill"] + + # Run 2 (deny-next-run): same persisted entry, provider now denies. + provider.denied_activate = {"demo-skill"} + sources = middleware._in_context_secret_sources(request, registry) + assert sources == [] + + +def test_tool_policy_reauthorizes_persisted_entries(monkeypatch, tmp_path): + """[P1 regression] allow-read-then-deny-next-run for allowed-tools: the + persisted entry no longer applies its declaration once skill:activate is + denied; an allowed skill keeps applying (partial deny does not poison).""" + from deerflow.agents.middlewares.skill_tool_policy_middleware import SkillToolPolicyMiddleware + + provider = _ActionAwareProvider(denied_activate=set()) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + + skills = {} + for name, allowed_tools in (("demo-skill", ("bash",)), ("ok-skill", ("web_search",))): + skill_dir = tmp_path / name + skill_dir.mkdir(exist_ok=True) + skill_file = skill_dir / "SKILL.md" + skill_file.write_text("# x", encoding="utf-8") + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skills[name] = SkillObject( + name=name, + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_file, + relative_path=Path(name), + category=SkillCategory.CUSTOM, + enabled=True, + allowed_tools=allowed_tools, + ) + + container_root = "/mnt/skills" + registry_entries = list(skills.values()) + + def _registry_path(skill): + return posixpath_normpath(skill.get_container_file_path(container_root)) + + paths = [_registry_path(skills["demo-skill"]), _registry_path(skills["ok-skill"])] + + monkeypatch.setattr( + SkillToolPolicyMiddleware, + "_storage", + lambda self: SimpleNamespace( + load_skills=lambda *, enabled_only: registry_entries, + get_container_root=lambda: container_root, + ), + ) + + middleware = SkillToolPolicyMiddleware( + available_skills={"demo-skill", "ok-skill"}, + slash_source_owner_token="test-token", + skill_authorization=resolved, + ) + # Both allowed: the union of both declarations applies. + provider.denied_activate = set() + allowed = middleware._allowed_names_for_paths(tuple(paths)) + assert "bash" in allowed and "web_search" in allowed + + # Deny-next-run for demo-skill only: its declaration stops applying, the + # still-allowed skill's declaration keeps applying. + provider.denied_activate = {"demo-skill"} + allowed = middleware._allowed_names_for_paths(tuple(paths)) + assert "bash" not in allowed + assert "web_search" in allowed + + # All denied: no active reference survives -> fail closed to builtins. + provider.denied_activate = {"demo-skill", "ok-skill"} + allowed = middleware._allowed_names_for_paths(tuple(paths)) + from deerflow.skills.tool_policy import ALWAYS_AVAILABLE_BUILTIN_TOOL_NAMES + + assert allowed == set(ALWAYS_AVAILABLE_BUILTIN_TOOL_NAMES) + + +# ── Async provider API on async execution paths (review round 5) ────── + + +def test_async_activation_path_uses_aauthorize(tmp_path, monkeypatch): + """awrap_model_call resolves skill:activate with aauthorize() on the loop — + a loop-affine provider whose sync API always fails still activates.""" + import asyncio + + from langchain_core.messages import HumanMessage + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = _middleware_for( + tmp_path, + monkeypatch, + ["demo-skill"], + available_skills={"demo-skill"}, + skill_authorization=resolved, + ) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + clone = _Request(**updates) + return clone + + request = _Request( + messages=[HumanMessage(content="/demo-skill hi")], + state={}, + runtime=None, + ) + captured = {} + + async def _handler(prepared): + captured["messages"] = list(prepared.messages) + return SimpleNamespace(ok=True) + + result = asyncio.run(middleware.awrap_model_call(request, _handler)) + + assert getattr(result, "ok", False) + # aauthorize answered; the failing sync authorize() was never consulted. + assert provider.async_calls == ["demo-skill"] + assert len(captured["messages"]) == 2 # activation reminder + original + assert any(is_slash_activation_reminder(m) for m in captured["messages"]) + + +def test_async_stamp_path_uses_aauthorize(): + """awrap_tool_call's skill-read stamp awaits aauthorize() — a loop-affine + provider still yields an entry (not a fail-closed denial marker).""" + import asyncio + + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + + request, message = _read_call_and_message("/mnt/skills/public/demo-skill/SKILL.md") + + async def _run(): + return await middleware._amaybe_stamp(message, request) + + stamped = asyncio.run(_run()) + assert provider.async_calls == ["demo-skill"] + assert SKILL_CONTEXT_ENTRY_KEY in stamped.additional_kwargs + assert SKILL_CONTEXT_DENIED_KEY not in stamped.additional_kwargs + + +def test_async_stamp_path_denies_via_aauthorize(): + import asyncio + + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + provider = _AsyncOnlyProvider(denied_activate={"demo-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + + request, message = _read_call_and_message("/mnt/skills/public/demo-skill/SKILL.md") + + async def _run(): + return await middleware._amaybe_stamp(message, request) + + stamped = asyncio.run(_run()) + assert provider.async_calls == ["demo-skill"] + assert SKILL_CONTEXT_ENTRY_KEY not in stamped.additional_kwargs + assert stamped.additional_kwargs.get(SKILL_CONTEXT_DENIED_KEY) is True + + +def test_describe_async_uses_aauthorize(): + """describe_skill's async invocation gates on aauthorize().""" + import asyncio + + provider = _AsyncOnlyProvider(denied_activate={"demo-skill"}) + tool = _describe_setup_for(provider, skills=("demo-skill", "other-skill")) + + async def _run(): + return await tool.ainvoke({"args": {"name": "select:demo-skill,other-skill"}, "name": "describe_skill", "type": "tool_call", "id": "async-call"}) + + result = asyncio.run(_run()) + content = result.update["messages"][0].content + assert "other-skill" in content + assert "demo-skill" not in content + assert sorted(provider.async_calls) == ["demo-skill", "other-skill"] + + +def test_async_tool_policy_uses_aauthorize(monkeypatch, tmp_path): + """SkillToolPolicyMiddleware's async hooks re-authorize persisted entries + with aauthorize() — the loop-affine provider's decision is respected.""" + import asyncio + + from deerflow.agents.middlewares.skill_tool_policy_middleware import SkillToolPolicyMiddleware + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + # aauthorize ALLOWS demo-skill: with correct wiring the skill's + # declaration applies ("bash" kept). If the async hook wrongly fell back to + # the sync API, the provider error would fail-closed and drop "bash". + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + + skill_dir = tmp_path / "demo-skill" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("# x", encoding="utf-8") + skill = SkillObject( + name="demo-skill", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("demo-skill"), + category=SkillCategory.CUSTOM, + enabled=True, + allowed_tools=("bash",), + ) + container_root = "/mnt/skills" + skill_path = posixpath_normpath(skill.get_container_file_path(container_root)) + + monkeypatch.setattr( + SkillToolPolicyMiddleware, + "_storage", + lambda self: SimpleNamespace( + load_skills=lambda *, enabled_only: [skill], + get_container_root=lambda: container_root, + ), + ) + + middleware = SkillToolPolicyMiddleware( + available_skills={"demo-skill"}, + slash_source_owner_token="test-token", + skill_authorization=resolved, + ) + request = SimpleNamespace(state={"skill_context": [{"name": "demo-skill", "path": skill_path}]}) + + async def _run(): + return await middleware._collect_activation_decisions(["demo-skill"]) + + decisions = asyncio.run(_run()) + assert decisions == {"demo-skill": True} + + # End to end through the async hook: the aauthorize-granted entry applies. + class _ToolsRequest(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _ToolsRequest(**updates) + + async def _identity(prepared): + return prepared + + tools_request = _ToolsRequest(state=request.state, tools=[SimpleNamespace(name="bash"), SimpleNamespace(name="read_file")], runtime=None) + filtered = asyncio.run(middleware.awrap_model_call(tools_request, _identity)) + + kept = [getattr(t, "name", None) for t in filtered.tools] + assert kept == ["bash", "read_file"] + + +# ── Canonical skill names: Skill.name != directory name ─────────────── +# +# Bundled skills may declare a name that differs from their directory +# (``skills/public/vercel-deploy-claimable/SKILL.md`` declares ``name: +# vercel-deploy``). Layer 1, slash activation, and describe_skill authorize +# the declared ``Skill.name``; every runtime gate that starts from a path +# must resolve it through the same registry before asking the provider. + + +def _mismatched_skill(tmp_path): + """Bundled-skill shape: directory name differs from the declared name.""" + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "vercel-deploy-claimable" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: Deploy stuff\n---\n# vercel-deploy\nbody", encoding="utf-8") + return SkillObject( + name="vercel-deploy", + description="Deploy stuff", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("vercel-deploy-claimable"), + category=SkillCategory.PUBLIC, + enabled=True, + allowed_tools=("bash",), + ) + + +def _patch_registry_storage(monkeypatch, middleware_cls, skills, container_root="/mnt/skills"): + monkeypatch.setattr( + middleware_cls, + "_storage", + lambda self: SimpleNamespace( + load_skills=lambda *, enabled_only: list(skills), + get_container_root=lambda: container_root, + ), + ) + + +def test_skill_read_stamp_authorizes_declared_name_not_directory(tmp_path, monkeypatch): + """[P2 regression] The stamp gate resolves the read path through the + registry and authorizes the declared Skill.name — never the directory + basename the path would suggest.""" + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + skill = _mismatched_skill(tmp_path) + provider = _ActionAwareProvider(denied_activate=set()) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + _patch_registry_storage(monkeypatch, ToolErrorHandlingMiddleware, [skill]) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + + request, message = _read_call_and_message("/mnt/skills/public/vercel-deploy-claimable/SKILL.md") + stamped = middleware._stamp_skill_read_metadata(message, request, tool_name="read_file") + + # The provider saw the declared name, and the read activates the skill. + assert provider.sync_calls == ["vercel-deploy"] + assert SKILL_CONTEXT_DENIED_KEY not in stamped.additional_kwargs + assert SKILL_CONTEXT_ENTRY_KEY in stamped.additional_kwargs + + +def test_skill_read_stamp_rbac_allowed_read_activates_mismatched_skill(tmp_path, monkeypatch): + """[P2 regression] RBAC allow: ["vercel-deploy"] — a read of the advertised + container path activates; allow: ["vercel-deploy-claimable"] (directory + name) does not, because the decision is keyed by the declared name.""" + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + skill = _mismatched_skill(tmp_path) + + def _stamped_with(provider): + resolved = _resolved_skill_authorization(provider, fail_closed=True) + _patch_registry_storage(monkeypatch, ToolErrorHandlingMiddleware, [skill]) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + request, message = _read_call_and_message("/mnt/skills/public/vercel-deploy-claimable/SKILL.md") + return middleware._stamp_skill_read_metadata(message, request, tool_name="read_file") + + allowed = _stamped_with(_rbac_provider({"user": {"skills": {"allow": ["vercel-deploy"]}}})) + assert SKILL_CONTEXT_ENTRY_KEY in allowed.additional_kwargs + assert SKILL_CONTEXT_DENIED_KEY not in allowed.additional_kwargs + + denied = _stamped_with(_rbac_provider({"user": {"skills": {"allow": ["vercel-deploy-claimable"]}}})) + assert SKILL_CONTEXT_ENTRY_KEY not in denied.additional_kwargs + assert denied.additional_kwargs.get(SKILL_CONTEXT_DENIED_KEY) is True + + +def test_async_stamp_path_authorizes_declared_name(tmp_path, monkeypatch): + """[P2 regression] The async stamp path canonicalizes off-loop and awaits + aauthorize() for the declared name — a loop-affine provider sees exactly + that target, and an allowed read still yields an entry (not a fail-closed + denial marker from a synchronous authorize() fallback).""" + import asyncio + + from deerflow.agents.middlewares.skill_context import SKILL_CONTEXT_DENIED_KEY, SKILL_CONTEXT_ENTRY_KEY + from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware + + skill = _mismatched_skill(tmp_path) + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + _patch_registry_storage(monkeypatch, ToolErrorHandlingMiddleware, [skill]) + middleware = ToolErrorHandlingMiddleware(app_config=_make_app_config(), skill_authorization=resolved) + + request, message = _read_call_and_message("/mnt/skills/public/vercel-deploy-claimable/SKILL.md") + + async def _run(): + return await middleware._amaybe_stamp(message, request) + + stamped = asyncio.run(_run()) + assert provider.async_calls == ["vercel-deploy"] + assert SKILL_CONTEXT_ENTRY_KEY in stamped.additional_kwargs + assert SKILL_CONTEXT_DENIED_KEY not in stamped.additional_kwargs + + +def test_async_slash_to_tool_policy_uses_declared_name(tmp_path, monkeypatch): + """[P2 regression] Composed async slash activation → tool-policy with + Skill.name != directory name: the decision maps are keyed by the declared + name resolved from the registry, the loop-affine provider answers every + check through aauthorize() (a map miss would fall back to the sync API and + fail closed, dropping the skill's tools), and the allowed skill keeps its + declared allowed-tools.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.agents.middlewares.skill_tool_policy_middleware import SkillToolPolicyMiddleware + + skill = _mismatched_skill(tmp_path) + storage = SimpleNamespace( + load_skills=lambda *, enabled_only: [skill], + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + _patch_registry_storage(monkeypatch, SkillToolPolicyMiddleware, [skill]) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + token = "test-token" + activation = SkillActivationMiddleware( + available_skills={"vercel-deploy"}, + skill_authorization=resolved, + slash_source_owner_token=token, + ) + policy = SkillToolPolicyMiddleware( + available_skills={"vercel-deploy"}, + slash_source_owner_token=token, + skill_authorization=resolved, + ) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + run_context: dict = {} + + async def _identity(prepared): + return prepared + + # Turn 1: the user slash-activates /vercel-deploy. The activation + # middleware resolves the skill by its declared name and records the + # canonical container path on the run context. + activation_request = _Request( + messages=[HumanMessage(content="/vercel-deploy deploy it")], + state={}, + runtime=SimpleNamespace(context=run_context), + ) + prepared = asyncio.run(activation.awrap_model_call(activation_request, _identity)) + assert any(is_slash_activation_reminder(m) for m in prepared.messages) + + # Turn 2 (tool loop): the tool-policy middleware picks the slash source up + # from the shared run context and re-authorizes before applying the + # skill's allowed-tools declaration. + policy_request = _Request( + messages=[], + state={}, + tools=[SimpleNamespace(name="bash"), SimpleNamespace(name="read_file")], + runtime=SimpleNamespace(context=run_context), + ) + filtered = asyncio.run(policy.awrap_model_call(policy_request, _identity)) + + assert provider.async_calls == ["vercel-deploy", "vercel-deploy"] + assert "vercel-deploy-claimable" not in provider.async_calls + kept = [getattr(t, "name", None) for t in filtered.tools] + assert kept == ["bash", "read_file"] + + +def test_async_secret_binding_canonicalizes_persisted_entry_paths(tmp_path, monkeypatch): + """[P2 regression] A persisted skill_context entry whose stamped name is + path-derived (directory != declared Skill.name) is canonicalized through + the registry before the async decision map is built: the loop-affine + provider answers for the declared name and the entry's declared secrets + still bind. A path-derived map key would miss and fall back to the sync + API from the worker thread, failing closed and dropping the binding.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + + skill = _mismatched_skill(tmp_path) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name="API_KEY", optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + + storage = SimpleNamespace( + load_skills=lambda *, enabled_only: [skill], + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"vercel-deploy"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + skill_path = posixpath_normpath(skill.get_container_file_path("/mnt/skills")) + # The stamped entry name is the directory name — the historical shape. + run_context = {"secrets": {"API_KEY": "secret-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="deploy the thing")], + state={"skill_context": [{"name": "vercel-deploy-claimable", "path": skill_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + assert provider.async_calls == ["vercel-deploy"] + assert "vercel-deploy-claimable" not in provider.async_calls + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"API_KEY": "secret-value"} + + +def test_async_secret_binding_preserves_prepass_failure(tmp_path, monkeypatch): + """[P2 regression, 4th instance of the sync-fallback class] A transient + prepass registry failure must not let the secret-binding resolution + silently recover with a fresh load: entry names resolved from a recovered + registry would miss the (empty) decision map and fall back to the + synchronous provider API — for a loop-affine provider a denial becomes a + fail-open allow. Entries bind nothing for that call. The slash source is + a run-scoped commitment validated at activation and does NOT consult the + decision map — it must survive the entry prepass failure unharmed.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + + def _skill(name: str, secret: str): + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / name + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text(f"# {name}", encoding="utf-8") + skill = SkillObject( + name=name, + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path(name), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name=secret, optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + return skill + + slash_skill = _skill("slash-skill", "SLASH_KEY") + entry_skill = _skill("entry-skill", "ENTRY_KEY") + + # Load order: #1 call-1 prepass snapshot, #2 call-1 slash activation, + # #3 call-1 fresh slash-source lookup, #4 call-2 prepass (FAILS), + # #5 call-2 fresh slash-source lookup (recovers; entries bind nothing + # from the sentinel, the slash binding survives). + load_calls = {"count": 0} + + def _flaky_load(*, enabled_only): + load_calls["count"] += 1 + if load_calls["count"] == 4: + raise RuntimeError("transient registry failure") + return [slash_skill, entry_skill] + + storage = SimpleNamespace( + load_skills=_flaky_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider(denied_activate={"entry-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=False) + middleware = SkillActivationMiddleware( + available_skills={"slash-skill", "entry-skill"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + entry_path = posixpath_normpath(entry_skill.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"SLASH_KEY": "slash-value", "ENTRY_KEY": "entry-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + async def _identity(prepared): + return prepared + + # Call 1: user slash-activates /slash-skill with the entry already in + # skill_context. Prepass succeeds; the aauthorize-DENIED entry binds + # nothing, the slash source binds. + first = _Request( + messages=[HumanMessage(content="/slash-skill go")], + state={"skill_context": [{"name": "entry-skill", "path": entry_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + asyncio.run(middleware.awrap_model_call(first, _identity)) + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"SLASH_KEY": "slash-value"} + + # Call 2 (tool loop): the entry prepass fails transiently (#4). The slash + # source must keep binding; the entry must bind nothing — a fresh-load + # recovery would fall back to the sync API and fail-open allow it. + second = _Request( + messages=[HumanMessage(content="continue")], + state={"skill_context": [{"name": "entry-skill", "path": entry_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + asyncio.run(middleware.awrap_model_call(second, _identity)) + + assert load_calls["count"] == 5 + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"SLASH_KEY": "slash-value"}, "entry must bind nothing after the prepass failure; the slash binding must survive it" + + +def test_async_secret_binding_resolves_entries_against_prepass_snapshot(tmp_path, monkeypatch): + """[P2 regression] Entry sources must resolve against the SAME registry + snapshot the decision map was keyed by. If the resolution re-loaded the + registry instead, a mid-step storage change (rename/uninstall) could + resolve the path to a name absent from the map — the sync-API fallback + divergence again, now without any load failure at all.""" + import asyncio + import dataclasses + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "entry-skill" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("# entry", encoding="utf-8") + skill = SkillObject( + name="entry-skill", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("entry-skill"), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name="ENTRY_KEY", optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + # What a FRESH load would return instead: the same container path now + # declaring a different name (mid-step rename). + renamed = dataclasses.replace(skill, name="renamed-skill") + + load_calls = {"count": 0} + + def _mutating_load(*, enabled_only): + load_calls["count"] += 1 + return [skill if load_calls["count"] == 1 else renamed] + + storage = SimpleNamespace( + load_skills=_mutating_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + # aauthorize allows "entry-skill" (the snapshot name) and denies + # "renamed-skill" (the fresh name); fail_closed so a wrong-name fallback + # fails closed and drops the binding. + provider = _AsyncOnlyProvider(denied_activate={"renamed-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"entry-skill", "renamed-skill"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + entry_path = posixpath_normpath(skill.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"ENTRY_KEY": "entry-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="plain task")], + state={"skill_context": [{"name": "entry-skill", "path": entry_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + assert provider.async_calls == ["entry-skill"] + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"ENTRY_KEY": "entry-value"}, "the entry must resolve against the prepass snapshot (allowed name), not a fresh load (denied name)" + + +def test_async_policy_preserves_prepass_registry_failure(monkeypatch, tmp_path): + """[P2 regression] A transient prepass registry failure must be preserved: + the worker-side filter must not retry storage — a successful retry would + resolve skills missing from the (empty) decision map and fall back to the + synchronous provider API from the thread, turning a loop-affine provider's + denial into a fail-open allow (fail_closed=false keeps the denied skill's + allowed-tools) or falsely denying permitted skills (fail_closed=true).""" + import asyncio + + from deerflow.agents.middlewares.skill_tool_policy_middleware import SkillToolPolicyMiddleware + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "demo-skill" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("# x", encoding="utf-8") + skill = SkillObject( + name="demo-skill", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("demo-skill"), + category=SkillCategory.CUSTOM, + enabled=True, + allowed_tools=("bash",), + ) + container_root = "/mnt/skills" + skill_path = posixpath_normpath(skill.get_container_file_path(container_root)) + + load_calls = {"count": 0} + + def _flaky_load(*, enabled_only): + load_calls["count"] += 1 + if load_calls["count"] == 1: + raise RuntimeError("transient registry failure") + return [skill] + + monkeypatch.setattr( + SkillToolPolicyMiddleware, + "_storage", + lambda self: SimpleNamespace( + load_skills=_flaky_load, + get_container_root=lambda: container_root, + ), + ) + + # fail_closed=False is the dangerous direction: an accidental sync-API + # fallback raises inside skill_activation_allowed and fail-opens into an + # allow, retaining the aauthorize-denied skill's tools. + provider = _AsyncOnlyProvider(denied_activate={"demo-skill"}) + resolved = _resolved_skill_authorization(provider, fail_closed=False) + middleware = SkillToolPolicyMiddleware( + available_skills={"demo-skill"}, + slash_source_owner_token="test-token", + skill_authorization=resolved, + ) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + state={"skill_context": [{"name": "demo-skill", "path": skill_path}]}, + tools=[SimpleNamespace(name="bash"), SimpleNamespace(name="read_file")], + runtime=None, + ) + + async def _identity(prepared): + return prepared + + filtered = asyncio.run(middleware.awrap_model_call(request, _identity)) + + kept = [getattr(t, "name", None) for t in filtered.tools] + # The prepass failure is preserved: exactly one storage attempt (no + # worker-side retry), no provider call at all, and the policy fails + # closed to framework-safe builtins — not the denied skill's "bash". + assert load_calls["count"] == 1 + assert provider.async_calls == [] + assert "bash" not in kept + assert "read_file" in kept + + +def test_async_model_call_without_skill_refs_skips_registry_scan(tmp_path, monkeypatch): + """[P2 regression] An ordinary authorization-enabled async model step (no + slash reference, no persisted entries, no supplied secrets) must not pay + a full skill-tree scan — the previously passive path needed no registry + I/O, and large or NFS-backed catalogs would add an uncached load_skills + before every LLM call.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + + load_calls = {"count": 0} + + def _counting_load(*, enabled_only): + load_calls["count"] += 1 + return [] + + storage = SimpleNamespace( + load_skills=_counting_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"demo-skill"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request(messages=[HumanMessage(content="an ordinary task, no slash reference")], state={}, runtime=None) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + assert load_calls["count"] == 0 + assert provider.async_calls == [] + + +def test_slash_secret_binding_uses_post_activation_registry(tmp_path, monkeypatch): + """[P2 regression] The slash source's secret binding resolves from a + fresh, post-activation registry — never the prepass snapshot. The + snapshot is taken before the aauthorize awaits; a skill whose declared + secrets change in that window would otherwise activate NEW_KEY content + while binding OLD_KEY (the just-activated skill misses the credential + it declares and receives one it no longer does).""" + import asyncio + import dataclasses + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "foo-skill" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: d\n---\n# foo", encoding="utf-8") + + def _skill(secret_name: str) -> SkillObject: + skill = SkillObject( + name="foo-skill", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("foo-skill"), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name=secret_name, optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + return skill + + old_version = _skill("OLD_KEY") + new_version = dataclasses.replace(old_version, required_secrets=[SimpleNamespace(name="NEW_KEY", optional=False)]) + + # A persisted (unrelated) entry makes the async prepass actually snapshot + # the registry; load #1 (prepass) sees the OLD declaration, the + # activation-era loads (#2 _resolve_activation, #3 slash binding) see NEW. + load_calls = {"count": 0} + + def _mutating_load(*, enabled_only): + load_calls["count"] += 1 + return [old_version] if load_calls["count"] == 1 else [new_version] + + storage = SimpleNamespace( + load_skills=_mutating_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"foo-skill", "other-skill"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + other_path = "/mnt/skills/custom/other-skill/SKILL.md" + run_context: dict = {"secrets": {"OLD_KEY": "old-value", "NEW_KEY": "new-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="/foo-skill go")], + state={"skill_context": [{"name": "other-skill", "path": other_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + # The activation read NEW; the binding must have followed it, not the + # prepass snapshot: NEW_KEY injected, OLD_KEY not. The slash decision + # itself came from aauthorize (not bypassed). + assert provider.async_calls == ["foo-skill"] + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"NEW_KEY": "new-value"} + + +def test_snapshot_entries_bind_while_slash_fresh_load_fails(tmp_path, monkeypatch): + """The two secret sources fail independently, each against its own + reference point: a transient failure of the fresh post-activation load + binds nothing for the slash source while the entry sources keep binding + from the prepass snapshot their decisions were keyed by.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + def _skill(name: str, secret: str) -> SkillObject: + skill_dir = tmp_path / name + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text(f"# {name}", encoding="utf-8") + skill = SkillObject( + name=name, + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path(name), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name=secret, optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + return skill + + slash_skill = _skill("slash-skill", "SLASH_KEY") + entry_skill = _skill("entry-skill", "ENTRY_KEY") + + # Load #1: prepass snapshot (both skills). Load #2: activation-era + # (_resolve_activation). Load #3: fresh slash lookup — FAILS transiently. + load_calls = {"count": 0} + + def _flaky_fresh_load(*, enabled_only): + load_calls["count"] += 1 + if load_calls["count"] == 3: + raise RuntimeError("transient fresh-load failure") + return [slash_skill, entry_skill] + + storage = SimpleNamespace( + load_skills=_flaky_fresh_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"slash-skill", "entry-skill"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + entry_path = posixpath_normpath(entry_skill.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"SLASH_KEY": "slash-value", "ENTRY_KEY": "entry-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="/slash-skill go")], + state={"skill_context": [{"name": "entry-skill", "path": entry_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + assert load_calls["count"] == 3 + # Both candidates were authorized via aauthorize (prepass succeeded); the + # fresh-load failure then gates only the slash source's resolution. + assert provider.async_calls == ["entry-skill", "slash-skill"] + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"ENTRY_KEY": "entry-value"} + + +def test_slash_era_dominates_entry_source_for_same_skill(tmp_path, monkeypatch): + """[construction-audit regression] When the same skill is BOTH the + slash-activated source (resolved from the fresh, post-activation registry) + and a persisted skill_context entry (resolved from the older prepass + snapshot), only the activation-era declaration binds. Unioning both + sources would inject a secret the skill no longer declares across the + prepass-to-activation window — the same staleness shape the slash-fresh + fix closed, arriving via the entry path. Mirrors the tool-policy rule: + explicit slash activation dominates for the run.""" + import asyncio + import dataclasses + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "foo" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: d\n---\n# foo", encoding="utf-8") + + def _skill(secret: str) -> SkillObject: + base = SkillObject( + name="foo", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("foo"), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(base, "required_secrets", [SimpleNamespace(name=secret, optional=False)]) + object.__setattr__(base, "secrets_autonomous", True) + return dataclasses.replace(base, required_secrets=[SimpleNamespace(name=secret, optional=False)]) + + old_version = _skill("OLD_KEY") + new_version = _skill("NEW_KEY") + + # Load #1 (prepass snapshot) sees the OLD declaration; the activation-era + # loads (#2 activation, #3 fresh slash lookup) see NEW. + load_calls = {"count": 0} + + def _mutating_load(*, enabled_only): + load_calls["count"] += 1 + return [old_version] if load_calls["count"] == 1 else [new_version] + + storage = SimpleNamespace( + load_skills=_mutating_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"foo"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + foo_path = posixpath_normpath(old_version.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"OLD_KEY": "old-value", "NEW_KEY": "new-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="/foo go")], + state={"skill_context": [{"name": "foo", "path": foo_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + # Only the activation-era declaration binds — not the union of both eras; + # the slash decision was authorized via aauthorize. + assert provider.async_calls == ["foo"] + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"NEW_KEY": "new-value"} + + +def test_sync_chain_secret_binding_uses_sync_api_and_canonical_names(tmp_path, monkeypatch): + """[Round-1 audit gap] The SYNC chain (wrap_model_call) with authorization + enabled resolves entry sources against a fresh registry and consults the + synchronous authorize() — the correct API there — with the canonical + declared name. The async snapshot path has regressions; this branch had + none, so a regression in the entry_registry=None fallback would pass the + whole suite.""" + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "vercel-deploy-claimable" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: d\n---\n# x", encoding="utf-8") + skill = SkillObject( + name="vercel-deploy", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("vercel-deploy-claimable"), + category=SkillCategory.PUBLIC, + enabled=True, + ) + object.__setattr__(skill, "required_secrets", [SimpleNamespace(name="API_KEY", optional=False)]) + object.__setattr__(skill, "secrets_autonomous", True) + + load_calls = {"count": 0} + + def _load(*, enabled_only): + load_calls["count"] += 1 + return [skill] + + storage = SimpleNamespace( + load_skills=_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _ActionAwareProvider(denied_activate=set()) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"vercel-deploy"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + skill_path = posixpath_normpath(skill.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"API_KEY": "v"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="plain task")], + state={"skill_context": [{"name": "vercel-deploy-claimable", "path": skill_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + def _handler(prepared): + return prepared + + middleware.wrap_model_call(request, _handler) + + # Two legitimate sync-API calls on the sync chain: the render-decision + # publication (path-keyed entry decisions) and the binding re-authorization. + assert provider.sync_calls == ["vercel-deploy", "vercel-deploy"] + assert provider.async_calls == [] + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"API_KEY": "v"} + # Two loads on the sync chain with active entries: the render-decision + # publication scan and the binding's fresh registry. + assert load_calls["count"] == 2 + + +def _durable_render_setup(tmp_path, monkeypatch, *, provider): + """Composed activation(outer) -> durable(inner) pair over two skills.""" + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.durable_context_middleware import DurableContextMiddleware + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + made = [] + for name in ("demo-skill", "ok-skill"): + skill_dir = tmp_path / name + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text(f"# {name}", encoding="utf-8") + made.append( + SkillObject( + name=name, + description=f"Description for {name}", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path(name), + category=SkillCategory.CUSTOM, + enabled=True, + ) + ) + demo_skill, ok_skill = made + + load_calls = {"count": 0} + + def _load(*, enabled_only): + load_calls["count"] += 1 + return made + + storage = SimpleNamespace( + load_skills=_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + resolved = _resolved_skill_authorization(provider, fail_closed=True) + token = "test-token" + activation = SkillActivationMiddleware( + available_skills={"demo-skill", "ok-skill"}, + skill_authorization=resolved, + slash_source_owner_token=token, + ) + durable = DurableContextMiddleware( + skills_container_path="/mnt/skills", + skill_authorization=resolved, + entry_decisions_owner_token=token, + ) + entries = [ + {"name": "demo-skill", "path": posixpath_normpath(demo_skill.get_container_file_path("/mnt/skills")), "description": "d", "loaded_at": 0}, + {"name": "ok-skill", "path": posixpath_normpath(ok_skill.get_container_file_path("/mnt/skills")), "description": "o", "loaded_at": 1}, + ] + return activation, durable, entries, load_calls + + +def _rendered_skill_names(prepared) -> set[str]: + text = "\n".join(str(getattr(m, "content", "")) for m in prepared.messages) + return {name for name in ("demo-skill", "ok-skill") if f"- {name}" in text} + + +def test_durable_render_hides_denied_entries_async(tmp_path, monkeypatch): + """The model-visible 'Active skills' reminder stops advertising skills the + provider denies (async chain): the activation middleware publishes the + per-step path-keyed decisions from its prepass (zero extra registry + scans) and the durable renderer consumes them.""" + import asyncio + + from langchain_core.messages import HumanMessage + + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + activation, durable, entries, load_calls = _durable_render_setup(tmp_path, monkeypatch, provider=provider) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="plain task")], + state={"skill_context": entries}, + runtime=SimpleNamespace(context={}), + ) + + async def _inner(prepared): + return await durable.awrap_model_call(prepared, _async_identity) + + async def _async_identity(prepared): # noqa: ARG001 + return prepared + + prepared = asyncio.run(activation.awrap_model_call(request, _inner)) + + assert _rendered_skill_names(prepared) == {"ok-skill"} + assert load_calls["count"] == 1, "the renderer must reuse the prepass publication, not scan" + + +def test_durable_render_hides_denied_entries_sync(tmp_path, monkeypatch): + """Same filter on the sync chain: the activation middleware's sync hook + publishes the decisions (one scan + sync authorize per entry name).""" + from langchain_core.messages import HumanMessage + + provider = _ActionAwareProvider(denied_activate={"demo-skill"}) + activation, durable, entries, load_calls = _durable_render_setup(tmp_path, monkeypatch, provider=provider) + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="plain task")], + state={"skill_context": entries}, + runtime=SimpleNamespace(context={}), + ) + + def _inner(prepared): + return durable.wrap_model_call(prepared, _sync_identity) + + def _sync_identity(prepared): # noqa: ARG001 + return prepared + + prepared = activation.wrap_model_call(request, _inner) + + assert _rendered_skill_names(prepared) == {"ok-skill"} + assert load_calls["count"] == 1 + + +def test_durable_render_unfiltered_without_authorization(tmp_path, monkeypatch): + """Authorization disabled: nothing is published and the reminder renders + every persisted entry — the absent-is-permissive carrier contract.""" + import asyncio + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares.durable_context_middleware import DurableContextMiddleware + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + + activation, durable, entries, load_calls = _durable_render_setup(tmp_path, monkeypatch, provider=_ActionAwareProvider(denied_activate=set())) + # Rebuild both without authorization, sharing the fixture's storage/skills. + activation = SkillActivationMiddleware(available_skills={"demo-skill", "ok-skill"}, slash_source_owner_token="test-token") + durable = DurableContextMiddleware(skills_container_path="/mnt/skills") + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="plain task")], + state={"skill_context": entries}, + runtime=SimpleNamespace(context={}), + ) + + async def _inner(prepared): + return await durable.awrap_model_call(prepared, _identity) + + async def _identity(prepared): + return prepared + + prepared = asyncio.run(activation.awrap_model_call(request, _inner)) + + assert _rendered_skill_names(prepared) == {"demo-skill", "ok-skill"} + + +def test_lead_chain_wires_durable_render_filter(): + """The lead chain's DurableContextMiddleware receives the resolved skill + authorization AND the same owner token as the chain's activation + middleware, and is constructed AFTER it (publication must precede + consumption in the wrap order). Dropping either silently disables the + rendered-reminder filter — nothing else fails.""" + from deerflow.agents.lead_agent.agent import build_middlewares + from deerflow.agents.middlewares.durable_context_middleware import DurableContextMiddleware + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + + middlewares = build_middlewares({"configurable": {}}, app_config=_make_app_config(), model_name="gpt-4", skill_authorization=resolved, user_id="user-123") + + activations = [m for m in middlewares if isinstance(m, SkillActivationMiddleware)] + durables = [m for m in middlewares if isinstance(m, DurableContextMiddleware)] + assert len(activations) == 1 and len(durables) == 1 + assert durables[0]._skill_authorization is resolved + assert durables[0]._entry_decisions_owner_token == activations[0]._slash_source_owner_token + assert middlewares.index(activations[0]) < middlewares.index(durables[0]), "activation must wrap outside durable so the per-step decisions are published before the renderer reads them" + + +def test_subagent_chain_wires_durable_render_filter(): + """Same wiring contract for the subagent builder's DurableContextMiddleware.""" + from deerflow.agents.middlewares.durable_context_middleware import DurableContextMiddleware + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.agents.middlewares.tool_error_handling_middleware import build_subagent_runtime_middlewares + + provider = _rbac_provider({"user": {"skills": {"allow": "*"}}}) + resolved = _resolved_skill_authorization(provider, fail_closed=True) + + middlewares = build_subagent_runtime_middlewares(app_config=_make_app_config(), user_id="user-456", skill_authorization=resolved) + + activations = [m for m in middlewares if isinstance(m, SkillActivationMiddleware)] + durables = [m for m in middlewares if isinstance(m, DurableContextMiddleware)] + assert len(activations) == 1 and len(durables) == 1 + assert durables[0]._skill_authorization is resolved + assert durables[0]._entry_decisions_owner_token == activations[0]._slash_source_owner_token + assert middlewares.index(activations[0]) < middlewares.index(durables[0]) + + +def test_entry_decisions_carrier_is_redaction_listed(): + """The per-step entry-decisions run-context carrier is stripped by + ``redact_secret_context_keys`` — the redaction allowlist must enumerate + every run-context key the middlewares add (paths and decisions are not + secrets, but the carrier must never leak onto observable surfaces).""" + from deerflow.runtime.secret_context import redact_secret_context_keys, write_skill_entry_decisions + + context: dict = {"secrets": {"K": "v"}, "__skill_entry_activation_decisions": {"decisions": {}, "owner_token": "t"}, "keep": 1} + write_skill_entry_decisions(context, {"/mnt/skills/custom/foo/SKILL.md": True}, owner_token="t") + + redacted = redact_secret_context_keys(context) + assert "__skill_entry_activation_decisions" not in redacted + assert "secrets" not in redacted + assert redacted.get("keep") == 1 + # redacted, not suppressed: the source carrier itself is untouched. + assert context["__skill_entry_activation_decisions"]["decisions"] == {"/mnt/skills/custom/foo/SKILL.md": True} + + +def test_slash_dominance_holds_when_activation_era_declares_no_secrets(tmp_path, monkeypatch): + """[P2 regression, R10] The same-skill exclusion must be derived from the + authenticated slash activation identity, not only from successfully + bound slash sources. When the prepass snapshots the OLD declaration + (required-secrets: [OLD_KEY]) and the operator edits the skill to declare + no secrets during the await window, the slash source binds nothing + (required_secrets gate) — the stale entry view must STILL be suppressed, + or a secret the just-activated skill no longer declares keeps injecting.""" + import asyncio + import dataclasses + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "foo" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: d\n---\n# foo", encoding="utf-8") + + base = SkillObject( + name="foo", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("foo"), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(base, "secrets_autonomous", True) + old_version = dataclasses.replace(base, required_secrets=[SimpleNamespace(name="OLD_KEY", optional=False)]) + # The activation-era version declares NO secrets at all. + new_version = dataclasses.replace(base, required_secrets=[]) + + # Load #1 (prepass snapshot) sees OLD; #2 (activation) and #3 (fresh + # slash lookup) see the no-secrets version. + load_calls = {"count": 0} + + def _mutating_load(*, enabled_only): + load_calls["count"] += 1 + return [old_version] if load_calls["count"] == 1 else [new_version] + + storage = SimpleNamespace( + load_skills=_mutating_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"foo"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + foo_path = posixpath_normpath(old_version.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"OLD_KEY": "old-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + request = _Request( + messages=[HumanMessage(content="/foo go")], + state={"skill_context": [{"name": "foo", "path": foo_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + # The slash-activated skill declares no secrets now; its stale entry view + # must not inject OLD_KEY. + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) is None + + +def test_slash_dominance_anchors_on_path_across_midrun_rename(tmp_path, monkeypatch): + """[R10-fix audit regression] The same-skill exclusion must anchor on the + authenticated identity PATH, not the declared name: when the operator + renames the skill (same path, new declared name) during the await window + and the user activates by the NEW name, a name-anchored exclusion set + (holding only the new name) lets the stale entry — resolving the same path + to the OLD name in the snapshot — bind two eras of declarations.""" + import asyncio + import dataclasses + + from langchain_core.messages import HumanMessage + + from deerflow.agents.middlewares import skill_activation_middleware as activation_module + from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware + from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY + from deerflow.skills.types import Skill as SkillObject + from deerflow.skills.types import SkillCategory + + skill_dir = tmp_path / "foo" + skill_dir.mkdir(exist_ok=True) + (skill_dir / "SKILL.md").write_text("---\ndescription: d\n---\n# foo", encoding="utf-8") + + base = SkillObject( + name="foo", + description="d", + license=None, + skill_dir=skill_dir, + skill_file=skill_dir / "SKILL.md", + relative_path=Path("foo"), + category=SkillCategory.CUSTOM, + enabled=True, + ) + object.__setattr__(base, "secrets_autonomous", True) + old_version = dataclasses.replace(base, required_secrets=[SimpleNamespace(name="OLD_KEY", optional=False)]) + renamed_version = dataclasses.replace(base, name="bar", required_secrets=[SimpleNamespace(name="NEW_KEY", optional=False)]) + + load_calls = {"count": 0} + + def _mutating_load(*, enabled_only): + load_calls["count"] += 1 + return [old_version] if load_calls["count"] == 1 else [renamed_version] + + storage = SimpleNamespace( + load_skills=_mutating_load, + get_container_root=lambda: "/mnt/skills", + get_skills_root_path=lambda: tmp_path, + ) + monkeypatch.setattr(activation_module, "get_or_new_skill_storage", lambda **kw: storage) + + provider = _AsyncOnlyProvider() + resolved = _resolved_skill_authorization(provider, fail_closed=True) + middleware = SkillActivationMiddleware( + available_skills={"foo", "bar"}, + skill_authorization=resolved, + slash_source_owner_token="test-token", + ) + + identity_path = posixpath_normpath(old_version.get_container_file_path("/mnt/skills")) + run_context: dict = {"secrets": {"OLD_KEY": "old-value", "NEW_KEY": "new-value"}} + + class _Request(SimpleNamespace): + def override(self, **kwargs): + updates = dict(self.__dict__) + updates.update(kwargs) + return _Request(**updates) + + # The user activates by the NEW name; the persisted entry is the OLD read + # of the same path. + request = _Request( + messages=[HumanMessage(content="/bar go")], + state={"skill_context": [{"name": "foo", "path": identity_path}]}, + runtime=SimpleNamespace(context=run_context), + ) + + async def _identity(prepared): + return prepared + + asyncio.run(middleware.awrap_model_call(request, _identity)) + + # Only the renamed (activation-era) declaration binds — no union of eras. + assert run_context.get(ACTIVE_SECRETS_CONTEXT_KEY) == {"NEW_KEY": "new-value"} + + +def posixpath_normpath(path: str) -> str: + import posixpath + + return posixpath.normpath(path) + + +def is_slash_activation_reminder(message) -> bool: + from deerflow.agents.middlewares.skill_activation_middleware import is_slash_skill_activation_reminder + + return is_slash_skill_activation_reminder(message) diff --git a/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md b/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md index 93179d4e7c..74c5d3c84a 100644 --- a/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md +++ b/docs/plans/2026-07-10-pluggable-authorization-implementation-notes.md @@ -339,8 +339,9 @@ Phase 1 最低验证要求： - **兼容性：** `authorization.enabled: false` 时两条路径均为 no-op（路由返回全部模型， 解析返回原名）。匿名请求（user=None）不触发过滤。RBAC provider 的 `_RESOURCE_POLICY_KEYS` 已包含 `"model": "models"`，无需 schema 变更。 -- **证据：** `tests/test_models_authorization.py`（24 tests）覆盖 disabled/anonymous/ - RBAC allow/deny/wildcard/fail-closed/fail-open 路由场景，disabled/allowed/ +- **证据：** `tests/test_models_authorization.py`（26 tests）覆盖 disabled/anonymous/ + RBAC allow/deny/wildcard/fail-closed/fail-open 路由场景（含 provider-resolution-error + fail-closed/fail-open），disabled/allowed/ graceful-fallback/all-denied-fail-closed/all-denied-fail-open/custom-provider-list-vs-use/ no-usable-fallback 运行时场景，以及 `DeerFlowClient._ensure_agent` 的 model:use 强制 + None 默认解析 + disabled no-op 集成场景； @@ -349,6 +350,60 @@ Phase 1 最低验证要求： - **延期：** Skills、Sandbox 权限（Phase 3 后续 PR）；前端 effective-permissions 展示； management route 的 provider 迁移。 +### 2026-07-28 — Phase 3 / Skills authorization (activate) + +- **背景：** Phase 2A 合并后，技能仍然对所有已认证用户开放——`available_skills` + 仅由 agent config 白名单控制，不检查角色。RFC §9 Phase 3 要求覆盖 Skills 资源类型。 + 技能的 enforcement 与 Models 不同：技能没有 Gateway route，而是在 harness 层 + （lead agent assembly + subagent executor + slash-activation middleware）执行。 +- **决策（Layer 1 — 装配阶段）：** 新增 `filter_available_skills_by_authorization()` + 过滤技能名称集合（`set[str] | None`），而非 `Skill` 对象列表。在 lead agent + `_make_lead_agent` 中，该函数在 `_available_skill_names()` 之后、`_load_enabled_available_skills()` + 之前执行，使得 catalog（`describe_skill`）和 `SkillActivationMiddleware`（slash 激活） + 共享同一个已过滤的 allowlist。在 subagent `executor.py` 的 `_load_skills()` 中同样过滤 + `config.skills` 白名单。 +- **决策（Layer 2 — 运行时 slash 激活，2026-09-03 修订）：** 成员检查之外，显式 + slash 激活还执行动作级 `authorize("skill", "activate")` 检查。`SkillActivationMiddleware` + 新增 `skill_authorization` 参数（`ResolvedSkillAuthorization`：装配时解析一次的 + provider + principal + fail_closed），在 `_resolve_activation` 的成员检查之后调用 + `_activation_allowed()`；deny 返回与成员拒绝相同的用户文案（不泄露原因），provider + 异常按 fail_closed 拒绝 / fail_open 放行。三条链路（lead `build_middlewares`、 + `DeerFlowClient._ensure_agent`、subagent `build_subagent_runtime_middlewares`）均接线。 + 修订原因：Layer 1 的 `filter_resources` 是动作无关的可见性层，动作感知的自定义 + provider 可以"可见但拒绝激活"——原设计（仅成员检查）对该类 provider 不闭合。 + ~~`describe_skill` 与 in-context 秘密绑定保持在可见性层~~（2026-09-04 再修订： + `describe_skill` 与 skill-file-load 路径也执行动作检查，见下方 2026-09-04 日志； + in-context 秘密绑定经由"拒绝的读取不进 `skill_context`"传递性覆盖）。 +- **决策（候选集解析的 fail-closed）：** 当 `available_skills=None`（无 agent 级白名单） + 且 `_all_configured_skill_names` 抛错（storage I/O 错误）时，不静默返回 `None` + （会让所有技能绕过授权）。改为让 `_all_configured_skill_names` 抛异常，调用方按 + `fail_closed` 决策：fail-closed → `set()`（拒绝全部），fail-open → `None`（无限制）。 + 与 provider 错误的 fail-closed/open 处理对称。 +- **决策（通用过滤器）：** 新增 `filter_resources_by_authorization()` 在 `enforcement.py`， + 是 `filter_tools_by_authorization` 的泛化版本，适用于任何有 `name` 属性的资源（技能、模型等）。 +- **否决方案（2026-09-03 部分反转）：** 原方案否决在 `_resolve_activation` 中添加 + 运行时 `authorize("skill", "activate")` 检查，理由是需要将 provider + principal 传入 + middleware 构造函数并改变 `build_middlewares` 签名，且认为 Layer 1 过滤已覆盖激活路径。 + 该理由对动作无关的 RBAC provider 成立，但对动作感知的自定义 provider 不成立（可见 ≠ + 可激活，评审 P1 发现并复现）——现按上文 Layer 2 修订落地，"穿 provider/principal 进 + 构造函数"的成本被接受，与 `_authorize_model_name` 的第二重 `authorize("model", "use")` + 检查模式一致。仍然成立的部分：不为技能新增独立 Gateway route（技能管理路由保持 + `require_admin_user`，per §12 Q6）；~~`describe_skill` 不做动作检查（见 Layer 2 修订）~~ + （2026-09-04 再修订：describe 与 skill-file-load 均已加动作检查，见下方日志）。 +- **兼容性：** `authorization.enabled: false` 时 `filter_available_skills_by_authorization` + 是 no-op（返回原始 allowlist）。`available_skills=None`（无 agent 级白名单）+ 启用授权时， + 从 config 解析全部技能名并过滤。RBAC provider 的 `_RESOURCE_POLICY_KEYS` 已包含 + `"skill": "skills"`，无需 schema 变更。对 test mock（SimpleNamespace app_config）安全： + 使用 `getattr` + `is not True` 防御。 +- **证据：** `tests/test_skills_authorization.py`（45 tests，2026-09-04 两轮扩充）覆盖 disabled/RBAC allow/deny/ + wildcard/empty/provider-error-fail-closed-fail-open/internal-caller 场景，generic filter， + 候选集解析错误的 fail-closed/fail-open + 空配置无 bypass 回归，以及 Layer 2：动作级 + deny 阻断激活（lead 中间件单测 + client 装配接线 + subagent 链接线）、provider 异常 + fail-closed/fail-open、候选集复用 catalog loader（不再二次扫描）、effective user + 三处统一。既有 authz + skills 测试全部通过。 +- **延期：** Sandbox 权限（Phase 3 后续 PR，已由 #4911 落地）；前端 effective-permissions 展示； + management route 的 provider 迁移。Models 权限由并行 PR（#4540）处理。 + ### 2026-08-02 — Phase 3 / Sandbox authorization (execute) - **背景：** Phase 3 Models 合并后，sandbox 仍只由 config presence @@ -438,6 +493,92 @@ Phase 1 最低验证要求： event loop；loop-affine provider 在 `__init__` 调用 `asyncio.get_running_loop()` 并在 `aauthorize()` 验证仍是同一个 loop。 +### 2026-09-04 — Phase 3 / PR #4541 自主加载路径动作门控、bootstrap 装配收口与 subagent provider 复用 + +- **背景：** review（CHANGES_REQUESTED）发现三个缺口：① 动作级 `skill:activate` 只在 + 显式 slash 路径执行——动作感知 provider 可以让技能"可见"（`filter_resources` 放行） + 但拒绝激活，模型仍可 `describe_skill` → `read_file`，`DurableContextMiddleware` 把文件 + 记入 `skill_context` 后 allowed-tools 策略与自主秘密绑定全部生效，被拒的动作从未检查； + ② bootstrap 分支把未过滤的 `set(_BOOTSTRAP_SKILL_NAMES)` 传给 `build_middlewares` / + `apply_prompt_template`，且 `skill_setup.skill_names or None` 在策略拒绝 `bootstrap` 时 + 把空集转回 `None`，legacy 全量 prompt 会重新加载并宣传被拒技能；③ subagent + `_load_skills()` 与 `_create_agent()` 各自解析一次 provider——provider 明确不缓存， + 两层可能拿到不同实例/策略快照，违反"Layer 1/Layer 2 同实例"契约。 +- **决策（共享动作检查）：** `skill_filter.skill_activation_allowed(authorization, name)` + 成为唯一的动作检查实现（provider + principal + fail_closed 语义，异常按配置 + fail-closed/open）。slash 路径（`SkillActivationMiddleware._activation_allowed` 委托）、 + `describe_skill`（匹配集过滤，全部被拒时与"无匹配"不可区分以避免泄露原因）、 + skill-file-load 盖章路径三处共用，防止语义漂移。 +- **决策（自主加载路径的 gate 位置）：** 盖章生产者 `ToolErrorHandlingMiddleware + ._stamp_skill_read_metadata` 是单一收口点——`skill_context` 条目、`SkillToolPolicyMiddleware` + 的 allowed-tools、`_in_context_secret_sources` 的秘密绑定全部以该盖章为源头。动作被拒的 + 读取改盖 `SKILL_CONTEXT_DENIED_KEY` 拒绝标记（而非条目元数据），`extract_skills` 见标记 + 即跳过且不产生 "missing skill read metadata" 告警。`skill_authorization` 经 + `_build_runtime_middlewares` → `build_lead_runtime_middlewares` / + `build_subagent_runtime_middlewares` 穿入。文件内容本身仍可读——可见性是 Layer 1 的 + 决定（与 sandbox projection 同语义），被拒的是"激活"（策略/秘密/持久上下文）。 +- **决策（bootstrap 装配）：** bootstrap 分支全程使用过滤后的 `available_skills` + （拒绝时为空集：激活中间件成员门 = 不可激活任何技能；legacy prompt 路径的空 allowlist + 分支返回空）；`skill_names` 仅在 deferred discovery 开启时保留（含空集），关闭时保持 + `None`（legacy 渲染）——允许路径行为不变。 +- **决策（subagent provider 复用）：** `SubagentExecutor._resolve_skill_authorization()` + 按执行器实例记忆化（executor 每次任务派发新建），Layer 1 filter（`authorization=`）、 + 激活中间件、describe 盖章、read 盖章四点共用同一 `ResolvedSkillAuthorization`。 +- **否决方案：** 不在 `read_file` 工具本体 gate（需要路径→技能名映射且影响非技能读取）； + 不在 `DurableContextMiddleware` gate（时序上晚于 `SkillToolPolicyMiddleware` 读 state， + 依赖 hook 顺序）；不在装配期过滤 catalog（会把 `` 可见性折叠进激活语义， + 且需 N 次 authorize 调用；运行期过滤每次 ≤ MAX_RESULTS 次）。 +- **证据：** `tests/test_skills_authorization.py` 45 tests（describe 动作门控 + provider + 错误 fail-closed/open + 禁用时全量；read 盖章拒绝/放行 + `extract_skills` 无告警跳过； + lead/subagent 链 stamping 接线；distinct-instance 工厂回归——`_load_skills` 与 + `_create_agent` 共享一个实例且仅解析一次）+ `test_authorization_enforcement.py` + bootstrap 三参数回归（允许/拒绝×deferred 开关）。突变验证：逐项还原六处修复， + 对应测试均失败。 +- **兼容性：** `authorization.enabled: false` 时三处 gate 均 no-op；`build_skill_search_setup` / + `build_describe_skill_tool` / `ToolErrorHandlingMiddleware` / 两个 runtime builder 的新 + 参数均默认 `None`；bootstrap 允许路径与 deferred 关闭路径行为不变。 + +### 2026-09-04（第二轮）— PR #4541 持久化条目复授权与异步 provider API + +- **背景：** review 第五轮发现两个缺口：① `skill_context` 跨 run 持久——条目在盖章时 + 授权过，但下一轮被 `SkillToolPolicyMiddleware`（allowed-tools）与 + `_in_context_secret_sources()`（秘密绑定）直接消费，不再查 `skill:activate`；provider + 从允许翻转为拒绝后，旧条目仍在施加策略与绑定秘密。② `skill_activation_allowed()` + 只调同步 `provider.authorize()`——异步执行路径因此用错 API：盖章在 event loop 上 + 同步调用、slash 激活与 describe 在 worker 线程同步调用；loop-affine provider 永远 + 收不到 `aauthorize()`，同步调用可能阻塞或失败，把允许错成 fail-closed 拒绝、把拒绝 + 错成 fail-open 放行。 +- **决策（异步助手）：** `skill_filter.skill_activation_allowed_async()` 与同步版同语义 + （同 fail-closed/open），经 `await provider.aauthorize()`，镜像 + `authorize_sandbox_execution_async` 的既有模式。 +- **决策（决策预计算模式）：** 异步 hook 在 event loop 上预计算 + `activation_decisions: dict[name, bool]`（slash 目标名 + 持久化条目名，一次批量 + aauthorize），传入 to_thread 中的阻塞处理函数；处理函数查映射，未覆盖的名字回退 + 同步检查（同步路径的正确 API）。三处落地：`SkillActivationMiddleware.awrap_model_call` + （决策贯穿激活与秘密绑定）、`ToolErrorHandlingMiddleware.awrap_tool_call`（`_amaybe_stamp` + 先 await 决策再盖章）、`SkillToolPolicyMiddleware.awrap_model_call` / `awrap_tool_call` + （条目名 + slash 来源名预计算）。 +- **决策（describe 双实现）：** `describe_skill` 用 `StructuredTool.from_function` + 同时挂同步函数与 coroutine——`invoke`（测试/同步链）走 `authorize()`， + `ainvoke`（异步 agent 执行）在 loop 上 `await aauthorize()`。 +- **决策（持久化条目复授权）：** 两个消费点在使用前复授权： + `_in_context_secret_sources` 对每个解析后的条目查动作决策，拒绝则跳过（不绑定秘密）； + `SkillToolPolicyMiddleware._active_skills_for_paths` 对每个解析后的技能查动作决策， + 拒绝则 `continue`（与 disabled / 出 allowlist 的技能同等跳过；全部被拒时沿用既有 + "no active reference authorized → fail-closed builtins" 语义）。`skill_authorization` + 新传入 `SkillToolPolicyMiddleware`（lead `build_middlewares` 与 subagent builder 两处）。 +- **否决方案：** 不在 `DurableContextMiddleware` 渲染时复授权（消费点直接读 state， + 渲染层过滤不覆盖 policy/secret 读取）；不在条目上缓存决策（持久化条目的正确缓存 + 粒度是"每次消费时重查"，与 live registry 复验同一模式）。 +- **证据：** `tests/test_skills_authorization.py` 45 tests：allow→deny-next-run 双回归 + （秘密绑定断源、allowed-tools 部分拒绝不株连、全拒 fail-closed builtins）、 + `_AsyncOnlyProvider`（同步 API 恒失败、aauthorize 可用）驱动四条异步路径断言 + （激活、盖章允许/拒绝、describe、tool policy——允许场景下正确接线保留技能声明， + 错回退同步 API 则 fail-closed 丢声明）。六项突变（两个复授权 gate + 四处异步预计算） + 逐一还原均有测试失败。 +- **兼容性：** 同步 hook 路径行为不变（决策映射仅异步构造）；`SkillToolPolicyMiddleware` + 新参数默认 `None`；`describe_skill` 对 `invoke` 调用方（既有测试）保持同步语义。 + ### 2026-09-17 — Phase 4 / PR #5489 Skills listing visibility (list / detail) - **背景：** Phase 4 PR 1（#5228 `/me` route permissions）与 PR 2（#5294 前端权限门控） @@ -698,6 +839,317 @@ Phase 1 最低验证要求： checkpoints 文档里的 `appcfg:531-575` / `appcfg:570-575` 行号引用在主线自己新增 prompt overlay 后已失准（当前 `_check_config_version` 位于 `appcfg:534-576`），不属本 PR 面内。 +### 2026-09-25 — PR #4541 规范技能名：授权目标统一为注册表 `Skill.name` + +- **背景：** review（ShenAC-SAC，P2）发现路径推导名与声明名不一致：bundled 技能目录名 + 可不同于 SKILL.md 声明的 `name`（`skills/public/vercel-deploy-claimable` 声明 + `vercel-deploy`），而 Layer 1、slash 激活、`describe_skill` 授权的都是声明名。 + ① read 盖章路径（同步 `_stamp_skill_read_metadata` / 异步 `_amaybe_stamp`）用 + `_skill_name_from_path()`（目录 basename）做 `skill:activate` 目标——RBAC + `allow: ["vercel-deploy"]` 下读取被误盖 `skill_context_denied`；② 异步决策预计算 map + 的键同样用路径推导名（slash 来源 + 持久化 `entry["name"]`，后者本身由 + `extract_skills` 按 path 盖章、必然是路径名），而消费点 `_active_skills_for_paths` / + `_in_context_secret_sources` 查的是注册表解析出的 `skill.name`——map miss 回退 + worker 线程里的同步 `authorize()`（`_AsyncOnlyProvider` 下直接 fail-closed 丢工具， + 且破坏上一轮消除同步回落的成果）。 +- **决策（共享注册表解析）：** 新增 `deerflow/skills/container_registry.py`： + `build_container_path_registry(storage)`（规范化容器 SKILL.md 路径 → live `Skill`， + `enabled_only=False`）与 `canonical_skill_name(registry, path)`。三个中间件 + （activation / tool-policy / tool-error-handling）统一经它把路径解析为声明名。 +- **决策（盖章路径）：** `ToolErrorHandlingMiddleware` 新增 `user_id`（经 + `_build_runtime_middlewares` → lead `build_middlewares` / subagent builder 穿入， + 与另两个技能中间件同一 storage 解析顺序——user-scoped 优先）。同步路径在 + `_canonical_skill_name()` 中解析（registry 失败或路径未解析 → 回退路径名，未解析路径 + 的下游消费本就按同一 registry 跳过）；异步路径 `asyncio.to_thread` 解析后于 loop 上 + `await skill_activation_allowed_async(...)`。 +- **决策（决策 map 键）：** `SkillToolPolicyMiddleware` 异步 hook 改三段式： + to_thread 加载 registry 并把策略路径规范名化（`_resolve_policy_registry`）→ loop 上 + `aauthorize()` 批量预计算（`_collect_activation_decisions(names)` 只收规范名）→ + to_thread 过滤并复用同一 registry（整 hook 一次 skill-tree 扫描）。slash 来源不再 + basename 推导，持久化条目一律取 `entry["path"]` 解析（不信任 `entry["name"]`）， + `_entry_names` 删除。`SkillActivationMiddleware` 同理：候选收集拆为 + `_candidate_activation_targets()`（slash 名——本就是注册表名 + 条目路径，loop-safe）， + `awrap_model_call` 先 to_thread 规范名化（`_canonical_names_for_paths`）再预计算。 +- **否决方案：** 不在 `extract_skills` 盖章时把 `entry["name"]` 改写为声明名（渲染层 + 展示名与路径一致有其意义，且历史条目仍需注册表解析，改写只修新条目不修存量）； + 不在 map 里同时预计算"路径名 + 声明名"两个键（掩盖键错配而非消除，同步回落仍可能 + 命中错误键）。 +- **证据：** 新增 5 个回归（`tests/test_skills_authorization.py` 45→50）：同步盖章断言 + provider 目标为声明名（`sync_calls == ["vercel-deploy"]`）；RBAC allow 声明名 → 激活 / + allow 目录名 → 拒绝（双向钉死经注册表解析）；异步盖章 `_AsyncOnlyProvider` 下 + `async_calls == ["vercel-deploy"]` 且条目盖章（同步回退会 fail-closed 出拒绝标记）； + 组合路径（async slash 激活 → tool-policy）`async_calls` 恰为声明名×2、`bash` 保留 + （回退同步 API 则 fail-closed 丢 `bash`）；持久化条目（路径名盖章的历史形态）经 + 注册表规范名化后秘密绑定仍生效（路径名键 → map miss → 同步回退 → 绑定丢失）。 + 三处突变（盖章路径名、tool-policy map 键、activation 条目名）逐一还原均有测试失败。 +- **兼容性：** 授权禁用时零新增开销（`skill_authorization is None` 不触达 registry）； + `ToolErrorHandlingMiddleware.user_id` 与两个 builder 的新参数默认 `None`； + 未解析路径回退路径名（与旧行为一致）；同步 hook 路径语义不变。 + +### 2026-09-26 — PR #4541 review（willem-bd，P2×2）：预扫描失败传递与空路径零扫描 + +- **背景：** 合入主线后的新一轮 review 指出规范名修复引入的两个缺口：① + `_resolve_policy_registry` 瞬时失败返回 `(None, [])`，worker 侧 + `registry is None` 会重试存储——重试若成功，恢复出的技能名不在（空的）决策 map + 里 → 回退 worker 线程的同步 `authorize()`：loop-affine provider 抛异常后 + `fail_closed=false` 变成放行（被拒技能的 allowed-tools 保留）， + `fail_closed=true` 变成误拒；② 授权开启时每个异步模型步都调用 + `_canonical_names_for_paths`，空 `entry_paths` 也触发全量技能树扫描——普通请求 + （无 slash、无条目、无 secrets）此前零注册表 I/O，大目录/NFS 下每步 LLM 前多一次 + 未缓存扫描。 +- **决策（失败传递）：** `_REGISTRY_LOAD_FAILED = object()` 三态哨兵（对齐同文件 + `_MISSING_POLICY_DECISION` 先例）：`_resolve_policy_registry` 失败时返回哨兵， + `_active_skills_for_paths` 见哨兵直接 `([], True)`（与首次加载失败同一 fail-closed + builtins 处置），不再静默重试。失败步的 builtins 决策经 refresh_decision 缓存 + 仅作用于本步；下一步模型调用预扫描重试存储即可恢复，无粘性状态。哨兵分支先于 + "No active skill references could be authorized" 告警返回——存储失败不产生该误导性 + 日志，异常日志（`_resolve_policy_registry` 内）是唯一信号。 +- **决策（空路径零扫描）：** `_canonical_names_for_paths` 开头 `if not paths: + return []`（单一 choke point，未来调用方自动受益）。普通异步模型步的存储调用数 + 归零（slash 解析在触达 storage 前返回 None；secrets 不存在则 + `_resolve_secret_bindings` 不加载注册表）。 +- **否决方案：** 不在失败后立即重试恢复（reviewer 给的第二选项）——多一次全量扫描 + 且复杂度更高，保留失败与既有"首次加载失败"语义逐字一致；不在调用点条件跳过 + `to_thread`（choke point 单点守卫才可被单点突变钉住）。 +- **证据：** `tests/test_skills_authorization.py` 50→52： + `test_async_policy_preserves_prepass_registry_failure`（首读失败、次读会成功的 + flaky storage + `_AsyncOnlyProvider(denied)` + `fail_closed=False`：恰一次 + storage 调用、零 provider 调用、builtins-only——旧行为下 worker 重试成功 → + 同步回退 → fail-open 保留 `bash`）；`test_async_model_call_without_skill_refs_ + skips_registry_scan`（storage 调用计数 == 0）。两项突变（哨兵还原为 None、 + 删除空路径守卫）逐一还原均有测试失败。 +- **兼容性：** 同步 hook（`registry is None` 自行加载）语义不变；`awrap_tool_call` + 缓存命中路径不触达哨兵；授权禁用时两条新路径均不存在。 + +### 2026-09-26（第二轮）— 第 4 实例收口：activation 秘密绑定改快照传递，类成员 grep 封闭 + +- **背景：** 上一节修复落地后的严格自查复现出同一 bug 类的第 4 个实例： + `SkillActivationMiddleware.awrap_model_call` 的条目预扫描瞬时失败返回 `[]` 并继续， + 线程内 `_resolve_secret_bindings` 在有 request secrets 时经 + `_load_skill_registry_by_path` **另一次独立加载**恢复——条目解析出的名字不在（空） + 决策 map → 回退 worker 线程同步 `authorize()` → `fail_closed=false` 时 + aauthorize 拒绝的技能秘密被 fail-open 绑定（复现：两次加载、零 aauthorize、 + 被拒秘密已注入）。slash 路径经复现实证免疫（名字来自消息解析，不经存储， + 预扫描失败也在 map 内）。 +- **决策（快照传递，对齐 policy 侧哨兵修复的形状）：** + `_canonical_names_for_paths` 返回 `(names, registry)`——名字与其来源快照一起返回； + 快照经 `_handle_model_request` 穿到 `_resolve_secret_bindings`，条目来源解析**复用该 + 快照**：决策 map 的键 = 快照解析的名字，map miss 在构造上不可能（连"同一步两次 + 加载不一致"的理论窗口——预扫描成功后存储被并发改名——也一并关闭）。加载失败返回 + `_REGISTRY_LOAD_FAILED` 哨兵：条目绑定归零（空注册表解析不出任何路径，构造保证）， + **slash 来源不受株连**（它不查决策 map，在激活时已验证，属 run 级用户承诺，照常 + 走新鲜加载）。同步链不传快照、行为不变。附带收益：有 secrets 的异步步从两次 + 注册表扫描降为一次（快照复用），`_load_skill_registry_by_path` 的新鲜度契约 + （"下一次模型调用即吊销"）仍然满足——快照取自本步开头。 +- **决策（类成员封闭 + 延期加固锚点）：** 全仓 grep 实证 + `skill_activation_allowed(` 同步调用点恰 4 处：stamping 同步路径（合法）、 + describe 同步实现（合法）、两个 `_activation_allowed`（activation / policy 中间件） + ——**异步可达的同步回退点有且仅有 2 个，本轮修复后全部关闭**。结构性加固 + （决策 map 类型化为 async-batch 语义对象，"异步路径 map miss = fail-closed + 响亮 + 日志，绝不静默同步回退"由构造保证；per-step 注册表快照经 run context 全链共享） + 作为后续工作延期——四实例证明调用方自觉已失效四次，但结构性重构会再开 N 轮 + review，先以实例收口 + 成员封闭 + 本锚点记录推进合并。 +- **否决方案：** 不用"失败标志只关门条目"（上一轮初步规格）——标志只堵失败扇窗， + 堵不住"预扫描成功但两次加载不一致"的窗口；快照传递同成本下把两类窗口都关成 + 构造不可能。不为空 `entry_paths` 跳过 `to_thread`（微秒级线程跳 vs 单点守卫的 + 突变可钉性，取后者；P3 有意放弃并记录）。 +- **证据：** `tests/test_skills_authorization.py` 52→54： + `test_async_secret_binding_preserves_prepass_failure`（两技能两调用：调用 2 预扫描 + 第 3 次加载失败——条目绑定归零且 slash 绑定存活；旧行为同步回退 fail-open 绑定 + 被拒的 ENTRY_KEY）、`test_async_secret_binding_resolves_entries_against_prepass_snapshot` + （第二次加载返回改名后的同路径技能：条目必须按快照名（allow）而非新鲜名（deny） + 解析）。三突变逐一还原必红：M1 哨兵改新鲜恢复（prepass failure 测试红）、M2 哨兵 + 株连 slash（同测试红）、M3 条目绕过快照（snapshot 测试红）。 +- **兼容性：** 授权禁用或无条目路径时 `awrap_model_call` 传 `None`， + `_resolve_secret_bindings` 走历史新鲜加载，行为不变；同步链完全不变； + policy 中间件注册表参数以 `_RegistryArg` 别名收编退化联合类型。 + +### 2026-09-27 — PR #4541 review（willem-bd R9）：slash 绑定参照点纠正与来源独立性 + +- **背景：** R8 的快照设计把 slash 来源也统一到了预扫描快照上——快照在 + `aauthorize()` await **之前**拍摄，slash 激活在其后重新加载当前技能；窗口内 + 技能声明秘密 OLD_KEY→NEW_KEY 变化时，激活展示 NEW 内容而绑定注入 OLD（刚激活的 + 技能拿不到它声明的凭据、反而拿到已不声明的）。R8 的"一步一载"附送优化在此翻车。 +- **决策（两个来源、两个参照点）：** slash 来源永远走**激活后的新鲜加载** + （`_load_skill_registry_by_path()`）——绑定必须与激活刚读到的内容一致；slash + 来源不查决策 map，快照一致性对它无价值。条目来源保留快照（map 键 = 快照名， + miss 构造不可能）；哨兵失败条目归零；`None`（同步链）回退同一新鲜注册表（同步 + authorize 在同步链是正确 API，无分叉）。两来源解析**相互独立**——新鲜加载瞬时 + 失败只归零 slash，条目继续按自己的快照绑定（审查自查发现初版修复把条目嵌在 + slash 守卫下，该场景会误杀条目绑定，已独立成行并由回归钉住）。 +- **复盘（为什么多轮自查仍漏）：** 五轮 review 中 reviewer 找的几乎全是"上一轮 + 修复新创造的面"；R9 的根源是快照统一这个**未被点名的附送优化**逃过了新面计价， + 以及"新鲜度"论证用了错误参照点（对照文档契约"下一步调用"而非"同调用内的激活 + 读"）且被写进文档后视为已封闭。已入 review-lessons 清单第 36–38 条（读取对× + 变化窗口×参照点；附送优化单独计价；已记录论证可再攻击）。 +- **四轴构造审计（举一反四，模拟 reviewer 方法）**：① 读取对×窗口—— + prepass↔activation 有意分离（各有参照点，代码注释记录）；activation↔binding + 为两次新鲜加载、窗口为文件读+哈希（无 await，µs 级），reviewer 措辞 + "fresh/activation-era metadata" 明示 fresh 可接受，且该窗口 PR 之前即存在， + 完全封闭需将 Skill 对象穿透 `_Activation`，记录为已知窗口不扩面；跨中间件 + 双快照（activation 与 policy 各自预扫描）无共享决策消费，良性。② 返回值× + 消费者——`(names, dict|sentinel|None)` 三值 × 唯一调用链全部处理，空 dict + 快照构造性绑定归零。③ I/O 失败文法——异步有密步加载 {#1 prepass, #2 激活, + #3 slash 新鲜}×失败：#1 失败钉（既有）、#3 失败钉（本轮新增）、#2 失败 + `_resolve_activation` 未捕获存储异常直接打断 run——**PR 之前既有**、不在本 + diff，记录不搭车。④ 边界——快照 dict 跨线程只读共享，无变异点。 +- **证据：** `tests/test_skills_authorization.py` 54→56： + `test_slash_secret_binding_uses_post_activation_registry`（预扫描成功+窗口内 + slash 声明 OLD→NEW：注入 NEW、不注入 OLD）；`test_snapshot_entries_bind_while_ + slash_fresh_load_fails`（新鲜加载瞬时失败：slash 归零、条目仍按快照绑定）。 + 突变 M4（slash 回退快照）、M5（条目重嵌 slash 守卫）逐一还原必红；上一轮双调用 + 测试加载计数按新序列更新（#5）。 +- **兼容性：** 同步链与授权禁用路径不变；"一步一载"附送收益放弃（回到 PR 前 + 加载计数），一致性优先；无新增 rider。 + +### 2026-09-27（第二轮自查）— 同技能双来源的"两个时代并集"与 slash 支配规则 + +- **背景：** 修复 R9 后按 reviewer 的构造审计法再读最终代码，构造出下一个场景并实证： + 同一技能既是 slash 来源（新鲜注册表，v2/NEW_KEY）又有持久条目（预扫描快照， + v1/OLD_KEY）时，两来源并集同时注入两个时代的钥匙（复现输出 + `{OLD_KEY, NEW_KEY}`）——OLD_KEY 正是"技能已不再声明却仍被注入"的形态， + 与 R9 同型、经条目路径到达。工具策略中间件已有先例语言"Explicit slash + activation dominates for the rest of that run"，秘密绑定对同一技能未对齐。 +- **决策（对齐先例）：** `_in_context_secret_sources` 新增 `exclude_names`—— + 已由 slash 来源绑定（激活时代）的技能名不再贡献条目视图。slash 是显式仪式且 + 解析自更新的注册表，同名条目视图只能使其过期或加宽；不同技能的条目照常 + 叠加（秘密是加法语义，与工具策略的排他语义不同——该不对称是既有设计，不动）。 +- **证据：** `test_slash_era_dominates_entry_source_for_same_skill`（快照 v1 + + 激活/新鲜 v2：仅注入 NEW_KEY）；突变 M6（移除排除）必红（并集 + `{OLD_KEY, NEW_KEY}` 回归）。套件 56→57。 +- **兼容性：** 仅同名支配；异名 slash+条目叠加行为不变（既有双调用回归覆盖）。 + +### 2026-09-27（双角色审查轮）— 同步链授权分支的覆盖缺口 + +- **背景：** 双角色（找问题/解决问题）审查第 1 轮：异步快照路径有 8 个回归，但 + **同步链 + 授权开启**（`wrap_model_call` + `skill_authorization` + 持久条目 + + secrets，走 `entry_registry=None` 新鲜回退分支）零覆盖——重构该分支的回归不会 + 红任何测试。行为先实证正确（规范名经同步 `authorize()`、绑定成功、恰一次 + 加载——同步 API 在同步链是正确契约）。 +- **决策：** 补 `test_sync_chain_secret_binding_uses_sync_api_and_canonical_names` + （`sync_calls == ["vercel-deploy"]`、`async_calls == []`、绑定生效、loads == 1）； + 突变 M7（`entry_registry=None` 回退改空 dict）必红。 +- **第 2 轮（换角度重扫）**：声明↔代码对齐逐条核（slash 不查 map、条目 map miss + 构造不可能、支配排除先于激活检查）；`tool_search` 确认在 + `ALWAYS_AVAILABLE_BUILTIN_TOOL_NAMES` 中（skill 策略层不破坏 deferral，既有 + 设计已覆盖）；秘密日志仅记录名字。无新发现，枚举空间记为已穷尽。 + +### 2026-09-27（第三轮自查，willem-bd 视角扫未审面）— user_id 接线回归缺口 + +- **背景：** 换到他从未审查的面：`container_registry.py` 新模块本身、`user_id` + 穿线的接线回归、Command 路径细节。代码接线正确（`_build_runtime_middlewares` + 把 `user_id` 传给 `ToolErrorHandlingMiddleware`，L514），但唯一接线测试 + （`test_lead_runtime_chain_forwards_skill_authorization_to_stamp_gate`）只断言 + `_skill_authorization`——**删掉 user_id 传递不会红任何测试**：stamp 门会静默 + 解析进程全局注册表，per-user 自定义技能的读路径回退路径推导名 → 错误授权 + 目标。subagent builder 同型缺口。 +- **决策：** lead 接线测试补 `user_id="user-123"` 断言；新增 + `test_subagent_runtime_chain_forwards_user_id_to_stamp_gate`。突变 M8（构造 + 调用去 user_id）双测试必红。 +- **其他面：** `container_registry.py` 全文精读（40 行）——键规范化双向、 + thread 纪律入档、`enabled_only=False` 理由明确，无发现。Command 路径 N-scan + 与 activation↔binding µs 窗口维持既有记录。 +- **证据：** 套件 58→59；ruff 干净。 + +### 2026-09-27（第四轮自查，willem-bd 视角扫未审面）— 持久条目的模型可见渲染不复授权 + +- **背景：** 未审面清单再推进：`DurableContextMiddleware` 渲染的 "Active skills" + 提醒直接来自 `state["skill_context"]`，**无任何 `skill:activate` 复授权**（grep + 实证）。provider 翻转为 deny 后：工具与秘密的消费点正确跳过（R6/R7 修复）， + `describe_skill` 也过滤被拒技能（R5 修复）——但模型可见的持久提醒仍宣传该技能的 + 名字与路径，直到条目离开 `skill_context`。 +- **定性：** 执行面无洞（模型重读 → stamp 门拒 → 不建新条目；声明工具/秘密均被 + policy/secret 门拦下）——这是**声明精度问题 + 表面不一致**：早期记录中"被拒的是 + 激活（策略/秘密/持久上下文）"的"持久上下文"一词过度声明，精确表述应为"被拒读取 + 不产生新持久条目；既有条目的**消费**（工具/秘密）复授权，但其**模型可见渲染** + 不过滤"。`DurableContextMiddleware` 属上游模块，渲染过滤记为 follow-up，不在本 + PR 扩面（对齐清单第 35 条）。 +- **证据：** grep 复授权关键词在该文件为空；渲染调用链 + `render_skill_context(state.skill_context)` 无条件透传。 +- **同轮其他未审面：** `release_policy_parameters` 契约（assembly_descriptor 的 + 可选鸭子类型；激活中间件发布的 `available_skills` 已是授权过滤后集合）——无发现。 + +### 2026-09-27（第五轮）— 渲染过滤落地：持久条目决策发布与 durable 消费 + +- **背景：** 第四轮将渲染过滤记为 follow-up 后，用户指令授权扩面实现。 +- **决策（发布/消费分离）：** `SkillActivationMiddleware` 每步发布**路径键**的 + ``skill:activate`` 决策到 run context（`__skill_entry_activation_decisions`， + 复用 slash source 的 owner-token 认证契约；已加入 `REDACTED_CONTEXT_KEYS`）： + 异步 hook 复用预扫描快照（零新增 I/O，失败/不可解析发布 False=隐藏）；同步 + hook 仅在有条件目时一次扫描 + 同步 authorize（被动步零 I/O，对齐 P2-B 先例）。 + `DurableContextMiddleware` 纯消费——按发布决策过滤**渲染副本**（state 不动）， + 自身不做任何 provider/storage 工作；未发布（授权禁用/无条目/非法载体）= 照旧 + 渲染（absent-is-permissive，与其他 run-context 载体一致）。lead 与 subagent + 两链均接线 `skill_authorization` + 共享 token。 +- **证据：** 3 个新测试（异步组合断言过滤生效且 `load_calls == 1`（渲染复用发布、 + 不自扫）；同步组合同断言；授权禁用渲染不变）。突变 M9（消费端不过滤）、 + M10（异步不发布）、M11（同步不发布）逐一必红。同步链既有测试的 sync_calls / + load 计数按新的合法双调用更新。 +- **兼容性：** 授权禁用路径零变化；`release_policy_parameters` 新增 + `skill_entry_render_filter` 布尔；state 不被渲染过滤触碰。 + +### 2026-09-27（第六轮自查）— 渲染过滤的链级接线回归 + +- **背景：** 渲染过滤落地后的新面上再执行"他式"检查：链级 wiring（token 配对、 + `skill_authorization` 传递、activation 先于 durable 的构造顺序——发布必须先于 + 消费）无任何测试；任一被静默删除，过滤失效且无测试变红（与 M8 同类）。 +- **决策：** 两个链级接线测试（lead `build_middlewares` / subagent + `build_subagent_runtime_middlewares`）：断言 durable 拿到同一 resolved 实例、 + token 与链上 activation 中间件相等、`index(activation) < index(durable)`。 + 突变 M12（lead 去 token）/ M13（subagent 去 authz）各自必红。 +- **证据：** 套件 62→64；受影响面 302 passed；ruff 干净。 + +### 2026-09-27（第二次 38 条全清单重跑）— 新面增验 + +- **重跑范围：** 渲染过滤落地后的 7 文件改动面。新增取证：新符号出现点枚举 + （`_publish_entry_decisions` 双调用点=同步/异步 hook；`_renderable_skills` + 单消费；write/read 单写单读；key 三用途=写/读/redaction 清单）；上游漂移 + **累计 21 个新提交**（`827acf51d..52a3e2564`）——提交前合并义务加重。 +- **发现并补齐 [第 5/24 条]：** 新载体 key 无 redaction 断言——补 + `test_entry_decisions_carrier_is_redaction_listed`（断言剥离 + "redacted, + not suppressed"：源载体保持完整）。 +- **维持：** M9–M13 突变全红记录、发布/消费构造保证、接线/顺序双测试、 + fail_closed=True（生产默认）方向覆盖。套件 64→65。 + +### 2026-09-28 — PR #4541 review R10（willem-bd，P2）：无秘密时代的支配排除 + +- **背景：** R9 修复推送后的新发现：同技能排除集此前从"成功绑定的 slash 来源" + 推导（`slash_bound = sources 名`）。当预扫描快照为旧声明（OLD_KEY）、operator + 在 await 窗口内把技能改为**不声明任何秘密**时：激活与新鲜 slash 查询看到新版， + `_resolve_registry_skill` 因 `required_secrets` 为空返回 None → slash 来源不进 + sources → 排除集为空 → 旧快照条目视图照旧把 OLD_KEY 注入——刚激活的技能已 + 不声明任何秘密却仍被注入。 +- **决策（身份推导）：** 排除集改为从**已认证的 slash 激活身份**（run context 里的 + slash source path，token 认证）推导：身份路径先在新鲜注册表、后在条目快照中 + 解析出名字（名字不因声明变化而失锚），加入排除集——与该技能当前是否绑定成功 + 无关。slash 身份在两处注册表都不可解析（技能已卸载）时不排除——沿用条目的 + 快照参照点语义。 +- **证据：** `test_slash_dominance_holds_when_activation_era_declares_no_secrets` + （快照 OLD、激活/新鲜=无秘密：`ACTIVE_SECRETS_CONTEXT_KEY is None`）；突变 M14 + （身份推导禁用，退回 sources 推导）必红。套件 65→66。 +- **兼容性：** 有秘密时代的支配行为不变（既有 dominance 回归覆盖）；无 slash + 身份时排除集来源不变。 + +### 2026-09-28（R10 修复自查）— 改名交错：身份锚定从名字升级为路径 + +- **背景：** 对 R10 修复本身执行清单第 39–41 条（用户点名"当前修改也要做设计/ + 方向处理"）。第 40 条（值→门→下游）命中：只枚举了 `required_secrets` 的 ∅， + 没枚举**声明 `name` 本身的变化**。复现坐实：operator 在 await 窗口内改名 + （同路径 foo→bar），用户按新名 `/bar` 激活——名字锚定的排除集只含 "bar"， + 快照条目把同一路径解析为旧名 "foo" → 不被排除 → `{OLD_KEY, NEW_KEY}` 两 + 时代并集，R10 的洞经改名存活。根源：条目与 slash 来源共享的稳定身份是 + **路径**；名字是每注册表版本可变的派生属性。 +- **决策（路径锚定 + 名字补充）：** `_in_context_secret_sources` 新增 + `exclude_paths`——条目规范化路径 == 已认证 slash 身份路径即排除（先于注册表 + 解析，不依赖任一版本叫它什么）；名字排除保留，覆盖同名异径遮蔽（custom + shadow public）。身份路径不可解析（已卸载）时路径排除仍然生效——身份是 + run 级承诺。 +- **证据：** `test_slash_dominance_anchors_on_path_across_midrun_rename` + （快照 foo/OLD_KEY，激活/新鲜 bar/NEW_KEY，按新名激活：仅 NEW 绑定）； + 突变 M16（仅路径排除禁用）必红。套件 66→67；受影响面 242 passed。 +- **复盘：** 本洞由"对刚写的修复立即执行新清单条目"抓出——第 41 条 + （重攻自身修复）的直接收益。 + ### 新记录模板 ```markdown