* Fix skill moderation parsing for Responses API content blocks
Normalize LangChain Responses API text blocks before parsing the security moderation decision, while preserving the existing fail-closed behavior for unavailable or invalid moderation results. Add regression coverage for mixed content blocks and document the compatibility boundary.
Constraint: Responses API AIMessage content is list-shaped while Chat Completions content is string-shaped
Rejected: Disable security scanning | would weaken the skill write safety boundary
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep moderation parsing provider-format tolerant without including reasoning or tool blocks in the decision payload
Tested: 27 security scanner tests; ruff check; ruff format check
Not-tested: Live moderation request against the configured external endpoint
* Reuse shared LLM response text normalization
Route skill moderation responses through the existing provider-format normalizer so only text and output_text blocks participate in JSON parsing. Strengthen regression coverage with reasoning and tool blocks that contain misleading text fields.\n\nConstraint: Responses API content is shared across multiple harness consumers\nRejected: Keep a private normalizer | duplicated provider-shape policy diverges and can reintroduce reasoning-block contamination\nConfidence: high\nScope-risk: narrow\nReversibility: clean\nDirective: Extend the shared normalizer when a new provider content shape is verified; do not add divergent local parsers\nTested: 118 related backend tests; regression test red against the previous parser; Ruff check and format check\nNot-tested: Live GitHub CLA status refresh
* Restore trusted external skill package loading
Skill discovery follows one-level package-directory symlinks, but activation path validation rejected the resolved external path. Restore that compatibility for configured custom-skill category roots while keeping file-level symlinks and deeper escapes blocked. Add regression coverage for local and user-scoped storage plus slash activation, and document the boundary.
Constraint: Existing skill discovery follows directory symlinks and operator-managed external packages must remain loadable
Rejected: Allow arbitrary resolved paths | would weaken the skill path trust boundary
Confidence: high
Scope-risk: moderate
Directive: Keep the final SKILL.md file symlink-free and preserve one-level category-root validation
Tested: 79 targeted skill storage, loader, slash activation, and user-scoped tests passed; Ruff check and format check passed; GitNexus staged change detection reported low risk
Not-tested: Real symlink activation on this Windows host lacks SeCreateSymbolicLinkPrivilege and is skipped
Related: Skill projection copies sources into sandbox-visible views
* Exercise real filesystem symlink boundaries in skill storage tests
Replace global Path.resolve/is_symlink mocks with real directory and file symlinks, preserving the Windows privilege skip. Add regression coverage for deeper custom-root escapes and symlinks under non-custom categories so the one-level allowance remains explicit.
Constraint: Symlink creation requires SeCreateSymbolicLinkPrivilege on some Windows runners
Rejected: Keep global path-method mocks | they validate the mock behavior rather than filesystem semantics
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep security-boundary tests on real filesystem primitives; skip only when the runner lacks symlink privilege
Tested: 76 targeted loader/storage/slash tests; Ruff check; Ruff format check
Not-tested: Windows symlink-enabled execution on this host
Related: #4936
* Pin the actual nested symlink escape boundary
Place the second symlink below a real custom package directory so the test reaches the one-level relative-parent guard instead of returning early on a non-symlink parent. Keep the public-category rejection coverage unchanged.
Constraint: The security boundary depends on both symlink depth and category root
Rejected: Link the outer package directory directly | the parent is not a symlink at validation time, so the depth guard is never evaluated
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep this regression tied to the exact relative_parent.parts depth check
Tested: Targeted storage, loader, and slash suites; GitNexus staged detection
Not-tested: Symlink-enabled execution on this Windows host
Related: #4936
* Make the nested symlink regression reach the depth guard
The test now validates the SKILL.md directly through the nested symlink, so the symlink is the immediate parent and the relative-parent depth check is executed.
Constraint: Windows test execution may skip when symlink privilege is unavailable
Rejected: Keep the extra nested path segment | it bypasses the symlink-depth guard through an early return
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Mutation tests must fail when the depth restriction is removed
Tested: Targeted test (skipped on this Windows host without symlink privilege); Ruff check and format check
Not-tested: Real symlink execution on Windows; Linux CI will exercise the case
Related: #4936
* Keep sandbox projections fresh for linked external skill packages
The storage layer intentionally accepts one-level custom package-directory symlinks, but projection freshness previously hashed only the link inode. Follow the permitted target tree during custom and legacy source-signature scans so edits to SKILL.md, scripts, references, or assets trigger a rebuild before sandbox use.
Constraint: Preserve the existing one-level custom/legacy symlink boundary and do not follow public, integration, nested, or file symlinks
Rejected: Invalidate projections only from /api/skills/reload | sandbox acquisition must also detect edits made directly in external targets
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep target-tree traversal limited to the storage paths that explicitly permit external package-directory links
Tested: 77 projection, user-scoped storage, and lifecycle tests passed; Ruff check and format check passed; git diff --check passed; GitNexus staged detection reported low risk
Not-tested: Real external symlink execution on this Windows host without SeCreateSymbolicLinkPrivilege; existing tests skip that platform limitation
* fix(skills): fail closed on drifted projection namespace on all platforms
* test(skills): add regression test simulating swallowed unlink on drifted namespace
* fix(sandbox): project enabled skills into sandbox views
* fix(skills): keep projection mutations consistent
* fix(skills): fail closed on projection errors
* fix(skills): isolate per-scope failures during boot projection rebuild
rebuild_all_skill_projections() propagated any exception from the public
rebuild or from a single user's rebuild straight out of the gateway
lifespan startup, uncaught. A single broken user directory (bad
permissions, corrupted _skill_states.json, unreadable content) would
therefore abort gateway boot for every user, not just that one -
_rebuild_*_locked already fails closed internally (clears the view and
re-raises), so the boot loop only needed to stop treating that re-raise
as fatal.
Each scope's rebuild now fails closed independently and boot continues;
a scope left empty by a boot failure self-heals on the next sandbox
acquire via ensure_skill_projections().
Also patches deerflow.skills.projection.rebuild_all_skill_projections in
the memory-flush lifespan test fixture, matching the two sibling
fixtures in the same file — this call is now on the lifespan startup
path and the fixture's minimal SimpleNamespace config predates it.
* test(skills): update authz test for the projection-aware public toggle
_persist_shared_skill_state (introduced earlier in this branch) reads
the shared extensions_config.json fresh from disk under the projection
lock instead of through the cached get_extensions_config() singleton -
that's the whole point of the fix (stale worker caches must not clobber
another worker's concurrent update). The name no longer exists on the
skills router module, so the test's monkeypatch of it started raising
AttributeError instead of exercising the endpoint.
The mock storage in this test isn't a real LocalSkillStorage instance,
so _persist_shared_skill_state's projection-mutation branch is already
skipped (nullcontext) and it falls back to a fresh ExtensionsConfig()
for the nonexistent tmp config_path - no replacement monkeypatch needed.
* fix(sandbox): make skill projection ensure best-effort in acquire
acquire() called _ensure_skills_projection() directly, outside any
try/except, in both LocalSandboxProvider and AioSandboxProvider. Every
other skill-mount setup path in these providers has always caught
exceptions and logged a warning rather than failing sandbox acquire
outright (e.g. when config.yaml can't be resolved) - these two new call
sites broke that contract, so any projection failure (including simply
not having a config.yaml, as in CI's test environment) now failed
acquire() itself instead of just leaving skill mounts off.
_ensure_skills_projection now catches its own exceptions and returns
None; both providers' callers already tolerate that (a None projection
skips the skill-specific mounts, matching the existing degrade path)
after making _append_public_skill_mapping and the custom/legacy mount
block in LocalSandboxProvider explicitly None-safe.
Caught by running the full suite with config.yaml removed, matching
CI's environment - not caught locally because a real config.yaml was
present, masking the failure.
* fix(sandbox): make E2B skill projection mounts best-effort
_skill_projection_mounts called ensure_skill_projections with no guard,
unlike Local/AIO's _ensure_skills_projection. A raise propagated out of
_apply_mounts before the configured-mounts loop ran, so a skills
projection failure dropped the operator's own configured mounts too -
only caught by create()'s outer warning, with nothing applied at all.
Swallow here and return an empty mount list on failure, matching the
Local/AIO pattern: still fail-closed for skills, but no longer widens
the blast radius to unrelated configured mounts.
Review feedback from PR #4178.
* docs(skills): document projection trade-offs flagged in review
- _update_tree_digest: note the metadata-only (not content) hashing
trade-off and why runtime writes through this codebase are still
covered regardless (rebuild-under-lock + rename always changes inode).
- LocalSandboxProvider.acquire: note the acquire-time self-heal cost
(cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift).
- skill_projection_mutation: drop the no-op except-Exception-then-raise;
a raise from the mutation already propagates past the yield with the
view left cleared, no explicit re-raise needed.
- provisioner README: spell out that hostPath skills volumes require
the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node
or shared storage), and that the custom/legacy volumes' hostPath type
Directory (not DirectoryOrCreate) makes a violation of that assumption
a visible Pod-creation failure instead of a silent empty mount.
Review feedback from PR #4178.
* fix(skills): lazily repair user projections
* fix(skills): close projection review gaps
* fix(skills): refresh user projection enable state
* fix(skills): close projection review follow-ups
* fix(skills): preserve state across projection writes
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
