mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-30 00:16:20 +00:00
17 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
ajayr
|
7e95bef2e7
|
feat(mcp): per-user credential injection for shared MCP servers (#4868)
* feat(mcp): per-user credential injection for shared MCP servers
A single HTTP/SSE MCP server entry can now serve several users, each
authenticated to the remote service with their own credential. A server
opts in with a user_auth block mapping user ids to credential header
values ($ENV_VAR references supported):
"user_auth": {
"header": "Authorization",
"users": { "<user-id>": "$SERVICE_TOKEN_ALICE" }
}
The built-in user-scoped auth interceptor resolves the authenticated
runtime user on every tool call (request runtime -> ambient LangGraph
runtime -> auth config -> request-scoped user ContextVar) and rewrites
the configured header via request.override(), the same per-call
mechanism as the OAuth interceptor. It registers after OAuth in the
shared assembly so its per-user value wins the header when a server
declares both. The entry's static headers are used only for startup
tool discovery.
Fail-closed by default: an unmapped user - including the anonymous
default-user fallback - or a credential whose env reference resolved
empty gets an actionable ToolException instead of another user's
credential; on_missing: "passthrough" opts out per server. Combined
with the existing per-(user, thread) MCP session scoping this gives
credential isolation on shared servers.
Gateway API: user_auth.users values are masked in GET responses, and
PUT round-trips preserve stored credentials for masked values (same
contract as env/headers/oauth secrets); a masked value for a user id
not already stored is rejected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(mcp): address review — preserve stored user_auth sub-fields on partial PUT, warn-and-skip stdio, allow extras
Review findings from #4868:
1. A partial user_auth payload (e.g. {"enabled": false}) merged to
users={} and default on_missing, irreversibly wiping stored
credentials on PUT. The merge is now sub-field-aware via
model_fields_set — omitted sub-fields carry the stored values, an
explicitly sent users map still replaces (so full-round-trip removal
works), masked values still swap back for stored credentials.
2. user_auth on a stdio server was a silent no-op: rewritten headers go
to call meta, never a transport header, while deny errors still fired.
The interceptor builder now warns and skips non-sse/http servers,
matching the tool_call_timeout transport-mismatch convention.
3. McpUserScopedAuthConfigResponse now allows extra keys like the
harness-side model, and extras survive masking and merge, matching
the server-level model_extra handling.
Adds four regression tests (partial-PUT preservation, explicit-map
replacement, extras round-trip, stdio warn-and-skip).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(mcp): reject blank user_auth.header at the gateway
A blank header passed the gateway response model, was persisted, then
failed the harness-side ExtensionsConfig validator on reload — the PUT
returned 500 after the write and every later config load/startup failed
until the file was hand-edited. Mirror the harness non-blank validator
on McpUserScopedAuthConfigResponse so the PUT fails with 422 before
anything is written.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* style: ruff format extensions_config.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(mcp): harden the trust chain and masking around user-scoped credential selection
Address review round 4:
- Scrub client-supplied user_id from run context/configurable for
external callers in inject_authenticated_user_context, before every
early return, and restamp only from request.state.user. Now that
user_id selects which user's credential user-scoped MCP auth injects,
a forged value must not survive any future path that skips the
restamp. Internal callers (IM channels, scheduler) keep supplying
end-user identity as before (PR #3294 contract). Regression tests pin
both the scrub and that a forged body.context.user_id can never
resolve as another user through merge + inject ordering.
- Include the caller's own resolved user id in the fail-closed deny
message so operators can copy the exact users key (it differs by
deployment path), and document the key formats in the mcp.mdx doc.
- Mask sensitive extra keys inside user_auth on GET like server-level
extras, and swap masked sentinels back for stored values on PUT via
_merge_extra_value_preserving_masked.
- Extract the interceptor wrap loop into compose_tool_interceptors and
pin the security property functionally: an OAuth interceptor that
actually sets Authorization loses the final header to the per-user
credential through the same composition the session-pool path uses.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Aari
|
236a068e77
|
fix(docker): let the Gateway write extensions_config.json in production (#4852)
* fix(docker): let the Gateway write extensions_config.json in production AGENTS.md states extensions_config.json may be edited at runtime through the Gateway API, and the Gateway implements that for the MCP enable switch, PUT/PATCH /api/mcp/config and the skill update route. Two properties of the production compose stack made every one of those writes fail: - the file was mounted read-only, and - Docker mounts it as its own mount point, so the temp-file-plus-rename in atomic_write_extensions_config hit EBUSY. Linux refuses rename() over a mount point whether or not the mount is writable, so making the mount read-write alone is not enough. Mount it read-write and fall back to an in-place overwrite on EBUSY only. The fallback is deliberately non-atomic and says so in a warning; it is reached only where the atomic route cannot work, and any other errno still propagates. config.yaml stays read-only: no API writes it. docker-compose-dev.yaml mounts the whole project directory, so the destination is an ordinary file there and this never surfaced in development. * test(docker): parse mount options instead of matching a :ro suffix Docker's short-syntax options segment is comma-separated, so a read-only mount can legally be spelled ":ro,z" or ":z,ro" — common with SELinux relabelling. Matching the raw string for a ":ro" suffix reads those as writable, which silently defeats the guard: the writability assertion would pass on a read-only mount, and the config.yaml assertion would fail on a correctly read-only one. Parse the options segment and test membership instead, and cover the parser with the spellings that broke the suffix check. * fix(config): harden mutable extensions config |
||
|
Aari
|
47b258ebd7
|
feat(mcp): add ordinary durable task driver (#4690)
* feat(mcp): add durable task runtime foundation * fix(chart): sync embedded config version * fix(mcp): isolate task polls during shutdown * feat(mcp): track consecutive poll errors on mcp_tasks poll_attempt_count grows on every claim (successful polls included), so it cannot drive a failure backoff without misjudging normal long tasks. Add consecutive_poll_error_count: incremented when a claim is released after a poll error, reset to zero by any applied snapshot. The backoff/terminal policy that consumes it lands with the first concrete driver. * fix(mcp): harden durable task lifecycle * feat(mcp): add ordinary durable task driver * test(mcp): address durable task review feedback * fix(mcp): preserve submit tool descriptions * fix(mcp): bound remote task calls * fix(mcp): bound persisted task payloads * fix(mcp): preserve task tool error details * fix(mcp): enforce durable task boundaries * test(mcp): cover task config snapshot lifecycle --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
lllyfff
|
99c926b7bb
|
fix(mcp): bring-up has no timeout and externalized tool outputs are counted as undelivered artifacts (#4657)
* fix: bound MCP server bring-up timeouts and exclude externalized tool outputs from delivery verification Two related robustness fixes: 1. MCP server bring-up was unbounded. tool_call_timeout only covered session.call_tool(); tool discovery (subprocess spawn + initialize + tools/list) and persistent stdio session initialization could hang forever, blocking agent construction (and on the Gateway event loop, the whole process). Add a per-server session_init_timeout (default DEFAULT_MCP_SESSION_INIT_TIMEOUT = 60s, null disables) that bounds both discovery and pooled-session initialization. The session pool's existing cancellation handling tears down a session stuck mid-creation in its own task. 2. ToolOutputBudgetMiddleware externalizes oversized tool outputs into outputs/.tool-results/ (configurable tool_output.storage_subdir). The workspace-change scanner and run delivery verification counted those files as produced artifacts, so any run that externalized a tool output without also presenting a real artifact failed with "Artifact delivery incomplete". Exclude TOOL_RESULTS_DIRNAME via a shared constant (mirroring BROWSER_FRAMES_DIRNAME) and thread the configured storage_subdir through snapshot capture so both workspace-changes events and delivery verification stay clean. * review: enforce single-segment tool_output.storage_subdir; document discovery-timeout cleanup Address review feedback: 1. A custom tool_output.storage_subdir with a path separator (e.g. cache/tool-results) silently no-oped the workspace-scanner exclusion: os.walk yields one-segment dirnames, so a nested value never matched and its files were counted as produced artifacts again. ToolOutputConfig now validates storage_subdir as a single directory name (rejects separators, .., absolute, empty) with tests, so the exclusion is always sound. 2. The discovery-timeout path now documents why cancellation is safe, mirroring the session-init note: discovery runs inside the adapter's nested async context managers, and stdio_client's finally terminates the process tree (SIGTERM->SIGKILL on POSIX, process-tree on Windows), so a timed-out npx subprocess and its children are reaped rather than accumulating. * review: log session-init timeouts and align API response model default with runtime config Address second-round review feedback: 1. A session-init timeout raised TimeoutError without any log, unlike the discovery timeout which logs a WARNING. Wrap the bounded get_session in a try/except that logs the timeout (server name + seconds) and re-raises, so operators can diagnose tool-call failures caused by hung MCP sessions. 2. McpServerConfigResponse.session_init_timeout defaulted to None while McpServerConfig defaults to 60s: a server created via PUT /api/mcp/config without the field was persisted with null (no timeout) while the same server created in the config file got 60s. Align the response-model default to DEFAULT_MCP_SESSION_INIT_TIMEOUT so API-created and file-created servers behave the same; an explicit null still opts out. * review: narrow the discovery-timeout handler to the bounded wait_for path The except TimeoutError clause covered both the bounded wait_for branch and the bare discovery branch. With session_init_timeout opted out (None), a TimeoutError raised by discovery itself would hit the %.1f format with None: logging raises TypeError internally, the WARNING is silently dropped, and a --- Logging error --- traceback goes to stderr. Narrow the handler to wrap only the wait_for call, where the branch condition guarantees the timeout value is not None. A discovery-internal TimeoutError on the opted-out path now falls through to the generic failure handler and is reported as 'tool discovery failed' with exc_info. Covered by a regression test that asserts the skip is reported without any broken format. |
||
|
Felix Wang
|
e221bddb38
|
feat: support per-server MCP tool name prefixes (#4624)
* feat: support per-server MCP tool name prefixes * refactor: pass MCP connection config directly * fix: preserve unprefixed MCP tool names in session pool |
||
|
Nan Gao
|
abe0dfd8fa
|
fix(mcp): constrain stdio launcher args and env at the config API (#4617)
* fix(mcp): constrain stdio launcher args and env at the config API
The API-managed stdio allowlist validated only `command`. Since `npx`/`uvx`
exist to fetch and run code, allowlisting the command name alone named a
binary without constraining what that binary ran, so the restriction did not
match the intent stated in `_validate_mcp_update_request`'s own docstring:
reducing the blast radius of a compromised authenticated browser session.
Screen two more fields on the stdio path, shared by `PUT /api/mcp/config` and
the enable branch of `PATCH`:
- `_ARBITRARY_EXEC_ARGS` rejects the flags that make a launcher evaluate a
string (`-c`, `--call`, `-e`, `--eval`, `--print`, `--shell`, `--node-arg`,
`--node-options`), matching `--flag=value` as well as `--flag value`, for
every allowlisted command rather than just `npx`/`uvx`. `-p` is deliberately
absent: it means `--package` for npx and `--python` for uv.
- `_CODE_INJECTING_ENV_VARS` rejects env names that inject code at process
startup (`NODE_OPTIONS`, `LD_PRELOAD`, `PYTHONSTARTUP`, ...), which would
otherwise sidestep the argument check.
Remote transports skip both -- they spawn nothing. Rejection messages echo
only the normalized flag, never the caller's value.
This is defense in depth, not a trust boundary: `npx`/`uvx` fetch and execute
remote packages by design, so an admin can still point one at a package they
published. Gateway admin remains equivalent to code execution on the host, and
the code, backend/AGENTS.md, and README all say so explicitly to keep the
check from being mistaken for a boundary.
`npx`/`uvx` stay in the default allowlist: nearly every official MCP server is
`npx -y @modelcontextprotocol/server-*`, so dropping them would break the
primary UI flow for adding one.
Tests cover each rejected flag spelling, case-insensitivity, `python -c` under
an extended allowlist, env injection, and the PATCH enable path, plus nine
positive cases pinning that real-world `npx`/`uvx`/`python -m` invocations
still validate.
* fix(mcp): screen PYTHONPATH and interpreter short-flag clusters
Review follow-up on the stdio launcher screen.
`PYTHONPATH` bypassed the env denylist on the *default* allowlist: `site`
imports `sitecustomize.py` from any `sys.path` entry before the tool's
entry point runs, so a caller-controlled directory is code execution
under plain `uvx` (verified against the real launcher). `PYTHONHOME` is
the same class via a repointed stdlib. Both are now rejected.
`PYTHONSTARTUP` is inert for non-interactive launchers and stays only as
belt-and-braces, now documented as such.
`-p` was exempted for every command, but it is node's `--print` --- the
long spelling was blocked while the short one passed once an operator
extended the allowlist. Whole-token matching also missed combined short
options (`node -pe`, `perl -we`, `python -Ic`). Both rules now apply to
commands outside `{npx, uvx}`, which are interpreters rather than package
runners; scoping them that way leaves the default allowlist unchanged,
since npx/uvx do not cluster short options and their trailing arguments
belong to a third-party server's own CLI where `-name` is ordinary.
`LD_LIBRARY_PATH`/`DYLD_LIBRARY_PATH` are recorded as an accepted
residual: conditional on the process loading a shadowable library, and
legitimately set by native-dependency servers.
Still defense in depth, not a trust boundary.
* docs(mcp): record NODE_PATH as an accepted search-path residual
Reviewer asked whether NODE_PATH belongs in _CODE_INJECTING_ENV_VARS for
parity with NODE_OPTIONS/PYTHONPATH. It does not, and the mechanism in the
report does not hold: verified on node v22, NODE_PATH is searched *after* the
local node_modules chain (the resolver unshifts the requiring module's paths
ahead of it), so it cannot shadow an installed dependency, and ESM import
ignores it entirely. It can only supply a CJS module that would otherwise
fail to resolve.
That puts it with LD_LIBRARY_PATH as a conditional search path, not with the
unconditional startup execution every entry in the set provides. Widen the
residual note from native-library shadowing to the general class, name
NODE_PATH in it, and pin the boundary with a test so it reads as a decision.
* fix(mcp): scope the stdio arg screen to the launcher's option region
`_arbitrary_exec_arg` inspected every element of `args`, justified by the claim
that the launchers in scope treat the token after `--` as a package name. That
is inaccurate: only the *first* token after `--` is the package name, and with
or without the separator every token from the package name onward is handed to
the third-party server's own argv, where `-c` is routinely "config" and `-e`
"env". So `npx -y @scope/server -c config.json` was rejected even though npm
never parses that `-c`, and the positive cases stayed green only because they
were picked to dodge the denylist. Over-blocking, not a bypass -- a regression
against the PR's own "the primary UI flow must keep working" goal.
Screen the launcher's own option region instead. Finding that region needs each
launcher's option *arity*, because a value is not a positional:
`npx -p <pkg> -c '<command>'` runs the command -- `-p` is `npm exec`'s
`--package` (it overrides the global `--parseable` shorthand), so `<pkg>` is
its value and npm keeps parsing its own flags. Ending the region at the first
non-flag token, the obvious rule, walks straight past that and turns an
over-block into a real eval bypass. The tables are therefore generated, not
guessed: `_NPX_BOOLEAN_ARGS` from `@npmcli/config` (npm 10.9.4) with the `-p`
override applied, `_UVX_VALUE_ARGS` from `uvx --help` (uv 0.11.1).
The unknown-option default is deliberately opposite per launcher, following the
exec set rather than symmetry. npx owns real exec flags, so an unknown option
consumes a value and keeps the region open; npm errors on options it does not
define, so that direction cannot reject an invocation that would otherwise
work. uvx owns no string-eval flag at all, so its screen is a tripwire rather
than a control, and an unknown option consumes nothing -- which keeps uv's
large and growing boolean surface from over-blocking. uvx also drops the short
spellings from its exec set, since `-c` is uv's `--constraints <file>`: an
ordinary documented option the old screen rejected outright.
Short options are matched case-sensitively now, because their case selects a
different option -- npm reads `-C` as `--prefix`, and folding it onto `-c`
rejected it. Long spellings stay case-insensitive: npm accepts `--CALL` and
runs it. Commands outside the package-launcher table are interpreters rather
than package runners and keep the unchanged whole-args screen, including the
`-p` rule and cluster decomposition.
Verified against the real launchers rather than by reading their docs. For
every argument vector in the new tests the validator's verdict matches whether
`npx` actually executes the string or passes it through to the server, and the
bypass-guard cases were confirmed to fail under the naive first-non-flag-token
boundary before being pinned.
Reported by @willem-bd on #4617. Two further false positives found while
confirming it are covered here: `uvx -c constraints.txt <tool>` above, and
`docker run -i --rm -e KEY <image>` -- the canonical GitHub MCP server
invocation -- which an operator who extends the allowlist with `docker` still
hits, since the conservative non-package-launcher path is unchanged.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Huixin615
|
133a82c6c2
|
fix: isolate MCP server toggles from invalid peer configs (#4577)
* fix: isolate MCP server toggle updates * fix: write extensions config atomically * fix: normalize MCP transport aliases |
||
|
ly-wang19
|
4582a41347
|
fix(skills): offload blocking filesystem IO in update_skill and serialize writes (#3565)
* fix(skills): offload update_skill's blocking IO and share the config write lock Re-applied on top of main rather than merged: update_skill has since gained admin gating, user-scoped storage and a PUBLIC vs CUSTOM/LEGACY split, so the offload is applied per path. - PUBLIC: the extensions_config.json read-modify-write (path resolve, snapshot, merge, write, reload) moves to a worker thread via asyncio.to_thread. The payload is built from a snapshot so the cached singleton is never mutated in place while the write is still in flight. - CUSTOM/LEGACY: set_skill_enabled_state is offloaded; the non-user-scoped fallback takes the same shared-file RMW path as PUBLIC. - Both load_skills calls (and storage construction) are offloaded. The RMW lock now lives next to reload_extensions_config as get_extensions_config_write_lock() and is acquired by both the skills router and the MCP router, which performs the same read-modify-write on the same file. Previously each router held its own module-local lock, so once both sides offloaded, a PUT /api/mcp/config could run inside a skill toggle's read->write window and the later write would silently drop the other's change. The lock is keyed by the running event loop rather than being a module-level singleton: asyncio primitives bind to the first loop that awaits them, which makes a plain module-level lock unusable in a process that runs more than one loop. Adds a cross-router regression anchor asserting a skill toggle and an MCP update never overlap inside the RMW (max in-flight 1); it observes 2 when the routers use separate locks. * fix(config): own the extensions_config RMW lock from the worker thread An asyncio.Lock held around `await asyncio.to_thread(...)` protects only the awaiting task. If that task is cancelled the context manager releases the lock immediately while Python keeps running the worker thread, so a second skills or MCP writer could acquire it and operate on extensions_config.json concurrently with the first worker — reopening the lost-update window this was meant to close. The per-event-loop keying had a second hole: writers on different loops got different locks and so did not exclude each other at all. Replace it with a process-wide threading.Lock acquired *inside* the worker that performs the RMW (`_write_extensions_skill_state` and `_apply_mcp_config_update`), so ownership belongs to the thread doing the writing and is held until the write and reload actually finish, regardless of what happens to the caller. A threading.Lock also has no event-loop affinity. Adds a cancellation regression: the skills worker is paused inside the lock, its route task is cancelled, and the MCP writer is started — the MCP RMW must not enter until the skills worker is released. Against the previous asyncio-lock design this test fails with ['skills-enter', 'mcp-enter']. The two existing serialization tests instrumented by replacing the worker functions, which now bypasses the lock under test; they instead patch inside the real workers (each module's reload_extensions_config, the last step under the lock) so the production lock is exercised. --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
ly-wang19
|
289adcbb02
|
fix(mcp): offload blocking filesystem IO in MCP config update (#3552)
* fix(mcp): offload blocking filesystem IO in MCP config update update_mcp_configuration resolved the extensions config path, probed its existence, read the raw JSON, wrote the merged config, and reloaded it — all blocking filesystem IO on the event loop (PUT /api/mcp/config). The whole read-modify-write after the async admin check has no interleaved awaits, so it moves into one _apply_mcp_config_update helper dispatched via asyncio.to_thread; the masked response is built on the loop. The secret-preserving merge, error codes, and the stdio command allowlist are unchanged. Found via `make detect-blocking-io`. Same class as #3457 / #3529 / #3551. Add tests/blocking_io/test_mcp_router.py anchor, verified red->green under the strict Blockbuster gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): serialize concurrent config updates with a write lock Address review on #3552: offloading the read-modify-write to a worker thread dropped the implicit serialization the single-threaded event loop provided, so two concurrent PUT /api/mcp/config calls could interleave and clobber each other. Guard the offloaded RMW with a module-level asyncio.Lock to restore within-process atomicity (cross-process writers remain a separate, pre-existing concern). Add a serialization regression test (red->green: without the lock the tracked max concurrency exceeds 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address mcp config blocking io review --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Ryker_Feng
|
5ba25b06ec
|
feat(mcp): add MCP routing hints (#4004)
* feat: add MCP routing hints * test: isolate mcp routing prompt config * fix: address mcp routing review feedback |
||
|
Nan Gao
|
2b301e8211
|
fix(channels): harden runtime credential management APIs (#3581)
* fix(channels): harden runtime credential management APIs * fix(channels): address review feedback on credential hardening Follow-up to the runtime credential-hardening pass, resolving five review findings: - WeChat auth persistence now writes through a 0o600 NamedTemporaryFile + Path.replace instead of write_text-then-chmod, so the iLink bot_token is never briefly readable at umask defaults (mirrors ChannelRuntimeConfigStore). - The post-write chmod is split into its own try/except: a chmod failure on a filesystem without POSIX perms now logs at debug instead of masquerading as a "failed to persist" warning. - Extracted the three near-identical _require_admin_user helpers (mcp, channel_connections, channels) into a single require_admin_user(request, *, detail) in app/gateway/deps.py; each router supplies its own detail string. - Strengthened the runtime-config-store chmod coverage: a new test injects a temp-file chmod failure and asserts it is logged at debug while the destination is still owner-only (mutation-verified to fail if the chmod is dropped), plus a loose-pre-existing-file case. - Removed the unused _FakeRepo from the blocking-io test: its isinstance gate routes through the repo-less 503 path, so neither stub was ever invoked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huixin615
|
1896722e66
|
fix: add MCP tools cache reset endpoint (#3602)
* fix: add MCP tools cache reset endpoint * docs: clarify MCP cache reset scope |
||
|
greatmengqi
|
40a371b88c
|
fix(security): harden MCP config endpoint (#3425)
* fix(security): harden MCP config endpoint * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: greatmengqi <chenmengqi.0376@bytedance.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Eilen Shin
|
872079b894
|
docs: clean standalone LangGraph server remnants (#3301) | ||
|
sunsine
|
7ec8d3a6e7
|
fix(security): mask sensitive values in MCP config API responses (#2667)
* fix(security): mask sensitive values in MCP config API responses GET /api/mcp/config previously returned plaintext secrets including env dict values (API keys), headers (auth tokens), and OAuth client_secret/refresh_token. Any authenticated user could read all MCP service credentials. This commit masks sensitive fields in GET/PUT responses while preserving the key structure so the frontend round-trip (GET masked → toggle enabled → PUT) correctly preserves existing secrets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): address Copilot review on MCP config masking - Load raw JSON (un-resolved $VAR placeholders) as merge source instead of resolved config, preventing plaintext secrets from replacing $VAR placeholders on disk (Comment 2) - Preserve all top-level keys (e.g. mcpInterceptors) in PUT, not just mcpServers/skills (Comment 1) - Reject masked value '***' for new keys that don't exist in existing config, returning 400 with actionable error (Comment 3) - Allow empty string '' to explicitly clear OAuth secrets, while None means 'preserve existing' for safe round-trip (Comment 4) - Add 3 new tests for rejection, clearing, and edge cases (18 total) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |
||
|
-Astraia-
|
191b60a326
|
fix: issue 1138 windows encoding (#1139)
* fix(windows): use utf-8 for text file operations * fix(windows): normalize sandbox path masking * fix(windows): preserve utf-8 handling after backend split |
||
|
DanielWalnut
|
76803b826f
|
refactor: split backend into harness (deerflow.*) and app (app.*) (#1131)
* refactor: extract shared utils to break harness→app cross-layer imports Move _validate_skill_frontmatter to src/skills/validation.py and CONVERTIBLE_EXTENSIONS + convert_file_to_markdown to src/utils/file_conversion.py. This eliminates the two reverse dependencies from client.py (harness layer) into gateway/routers/ (app layer), preparing for the harness/app package split. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: split backend/src into harness (deerflow.*) and app (app.*) Physically split the monolithic backend/src/ package into two layers: - **Harness** (`packages/harness/deerflow/`): publishable agent framework package with import prefix `deerflow.*`. Contains agents, sandbox, tools, models, MCP, skills, config, and all core infrastructure. - **App** (`app/`): unpublished application code with import prefix `app.*`. Contains gateway (FastAPI REST API) and channels (IM integrations). Key changes: - Move 13 harness modules to packages/harness/deerflow/ via git mv - Move gateway + channels to app/ via git mv - Rename all imports: src.* → deerflow.* (harness) / app.* (app layer) - Set up uv workspace with deerflow-harness as workspace member - Update langgraph.json, config.example.yaml, all scripts, Docker files - Add build-system (hatchling) to harness pyproject.toml - Add PYTHONPATH=. to gateway startup commands for app.* resolution - Update ruff.toml with known-first-party for import sorting - Update all documentation to reflect new directory structure Boundary rule enforced: harness code never imports from app. All 429 tests pass. Lint clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: add harness→app boundary check test and update docs Add test_harness_boundary.py that scans all Python files in packages/harness/deerflow/ and fails if any `from app.*` or `import app.*` statement is found. This enforces the architectural rule that the harness layer never depends on the app layer. Update CLAUDE.md to document the harness/app split architecture, import conventions, and the boundary enforcement test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add config versioning with auto-upgrade on startup When config.example.yaml schema changes, developers' local config.yaml files can silently become outdated. This adds a config_version field and auto-upgrade mechanism so breaking changes (like src.* → deerflow.* renames) are applied automatically before services start. - Add config_version: 1 to config.example.yaml - Add startup version check warning in AppConfig.from_file() - Add scripts/config-upgrade.sh with migration registry for value replacements - Add `make config-upgrade` target - Auto-run config-upgrade in serve.sh and start-daemon.sh before starting services - Add config error hints in service failure messages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix comments * fix: update src.* import in test_sandbox_tools_security to deerflow.* Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: handle empty config and search parent dirs for config.example.yaml Address Copilot review comments on PR #1131: - Guard against yaml.safe_load() returning None for empty config files - Search parent directories for config.example.yaml instead of only looking next to config.yaml, fixing detection in common setups Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: correct skills root path depth and config_version type coercion - loader.py: fix get_skills_root_path() to use 5 parent levels (was 3) after harness split, file lives at packages/harness/deerflow/skills/ so parent×3 resolved to backend/packages/harness/ instead of backend/ - app_config.py: coerce config_version to int() before comparison in _check_config_version() to prevent TypeError when YAML stores value as string (e.g. config_version: "1") - tests: add regression tests for both fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update test imports from src.* to deerflow.*/app.* after harness refactor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> |