mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-11 14:38:38 +00:00
fix(gateway): gate github_token and disable_clarification on internal callers (#5338)
* fix(gateway): gate github_token and disable_clarification on internal callers `non_interactive` is honored only for internally-authenticated callers because it strips `ask_clarification` from the lead-agent toolset. The two sibling run-context keys reproduced that effect without the gate. `merge_run_context_overrides` forwarded `_CONTEXT_RUNTIME_ONLY_KEYS` regardless of `internal`, and `strip_internal_context_keys` scrubbed only `_CONTEXT_INTERNAL_CALLER_KEYS` -- so any session or PAT caller could set `disable_clarification` through `body.context`, or through the free-form `body.config` that `build_run_config` copies verbatim. That is not a milder flag than `non_interactive`: ClarificationMiddleware answers every clarification -- `risk_confirmation` included -- with "proceed without asking" instead of interrupting, and SandboxMiddleware reads the two keys as the same non-interactive signal. `github_token` rode the same path into `runtime.context`, where the bash tool exports it as `GH_TOKEN`/`GITHUB_TOKEN`, and a copy smuggled through `body.config['configurable']` reached the checkpoint store the context-only rule exists to avoid. Both keys are produced server-side by the channel run policies, which reach the Gateway over the internally-authenticated request channel, so gate them the same way: forward them only when `internal=True`, and scrub the union `_INTERNAL_ONLY_CONTEXT_KEYS` from both config sections for every other caller. Destination stays an orthogonal axis -- `_CONTEXT_RUNTIME_ONLY_KEYS` still land in `context` alone, never in checkpoint-persisted `configurable`. Regression coverage in tests/test_gateway_services.py pins both smuggling surfaces and replays the real start_run assembly order for a session caller and for an internal one, so the GitHub channel keeps carrying its minted token. * docs(changelog): record the internal-only run-context key gate (#5338) * docs(agents): keep the run-context note inside the AGENTS.md budgets The AG002 inherited-chain check failed at this head. The new backend section and the root scheduled-task sentence added 993 B to the root and backend guidance both the sandbox and middlewares chains inherit, pushing sandbox 6 B over the 98304 B hard limit and growing the middlewares chain, which main already exceeds by 155 B. An already-over chain is only tolerated while it does not grow, so the shared ancestors had to come back to their base size. Condensed the new material and removed prose the root file was duplicating: - The trust-boundary section keeps both gated surfaces, both helpers, the trust-vs-destination split, and the disable_clarification note in half the space. - The root scheduled-task bullet names all three internal-only keys and both smuggling surfaces while staying under its previous size. - Dropped the root `scheduler.recursion_limit` bullet, which restated backend/AGENTS.md:18 almost verbatim; its one unique fact (a YAML edit needs no Gateway restart) moved to that bullet. - Deduplicated the nginx routing sentence, which already deferred to the backend routing table, and tightened the waiver note's sequencing tail. Root and backend guidance now sit 50 B under their combined base size, so the sandbox chain returns to 97310 B and the middlewares chain no longer grows. Every file stays under its AG001 soft budget.
This commit is contained in:
Hyeonsang Cho
GitHub
parent
452d09b96b
commit
556975f284
44
AGENTS.md
44
AGENTS.md
@ -31,24 +31,21 @@ A single `make dev` / Docker stack runs four cooperating services:
|
||||
| **Frontend** | `3000` | Next.js web interface |
|
||||
| **Provisioner** | `8002` | Optional — only when sandbox is configured for provisioner/K8s mode |
|
||||
|
||||
Nginx is the single public entry: it serves the frontend and proxies `/api/langgraph/*`
|
||||
to the Gateway's LangGraph runtime, rewriting it to Gateway's native `/api/*` routes; all
|
||||
other `/api/*` go straight to the Gateway REST routers. See
|
||||
[backend/AGENTS.md](backend/AGENTS.md) for the runtime and router detail.
|
||||
It compresses HTML and configured textual assets, while deliberately leaving SSE,
|
||||
fonts, images, audio, and video uncompressed at the proxy layer.
|
||||
Nginx is the single public entry: it proxies `/api/*` to the Gateway, rewriting
|
||||
`/api/langgraph/*` onto the Gateway's native routes, and serves the frontend — see
|
||||
[backend/AGENTS.md](backend/AGENTS.md) for the runtime and router detail. It compresses
|
||||
HTML and configured textual assets, deliberately leaving SSE, fonts, images, audio, and
|
||||
video uncompressed at the proxy layer.
|
||||
|
||||
Both compose files publish that entry as `"${BIND_HOST:-127.0.0.1}:${PORT:-2026}:2026"`
|
||||
— **loopback by default**, matching the README's documented deployment model. A bare
|
||||
`"${PORT}:2026"` binds `0.0.0.0`, which does not.
|
||||
The root `PORT` value is Docker ingress configuration only; local orchestration pins
|
||||
Next.js to `3000` so loading `.env` cannot make `make dev` wait on the wrong port.
|
||||
Nginx itself listens `default_server` on IPv4+IPv6 and the
|
||||
Gateway binds `0.0.0.0:8001` inside the container on purpose — both are container-
|
||||
internal; the published nginx port is the entire external surface, and the Gateway's
|
||||
`8001` is deliberately not published. Any new published port needs an explicit bind
|
||||
address; `backend/tests/test_compose_default_bind_host.py` pins this for every service
|
||||
in both compose files.
|
||||
— **loopback by default**, matching the README's documented deployment model; a bare
|
||||
`"${PORT}:2026"` binds `0.0.0.0`, which does not. The root `PORT` value is Docker ingress
|
||||
configuration only; local orchestration pins Next.js to `3000` so loading `.env` cannot
|
||||
make `make dev` wait on the wrong port. Nginx listening `default_server` on IPv4+IPv6 and
|
||||
the Gateway binding `0.0.0.0:8001` are container-internal on purpose: the published nginx
|
||||
port is the entire external surface. Any new published port needs an explicit bind
|
||||
address; `backend/tests/test_compose_default_bind_host.py` pins this for every service in
|
||||
both compose files.
|
||||
|
||||
## Repository Map
|
||||
|
||||
@ -105,19 +102,16 @@ Skill quality review note:
|
||||
edits from their head revision, but only the manifest from the trusted base
|
||||
revision can suppress that run. Entries match one error finding exactly,
|
||||
include the reviewed file's SHA-256 and an expiry date, remain visible in CI
|
||||
output, and can never waive blocker findings. An entry may also preapprove a
|
||||
bounded list of future full-file SHA-256 values. Those hashes become effective
|
||||
only after the manifest change lands in the trusted base. Adding or changing a
|
||||
waiver and relying on it therefore requires two steps: merge the reviewed
|
||||
manifest change first, then update the affected public skill in a later pull
|
||||
request. After that skill change lands, promote its hash to `file_sha256` and
|
||||
remove the consumed preapproval in a follow-up manifest cleanup.
|
||||
output, and can never waive blocker findings. An entry may also preapprove
|
||||
future full-file SHA-256 values, effective only once the manifest change lands
|
||||
in the trusted base — so relying on a waiver takes two merges: the manifest
|
||||
first, the skill change after, then promote the consumed hash to `file_sha256`
|
||||
in a follow-up cleanup.
|
||||
|
||||
Scheduled-task note:
|
||||
- The scheduled-task MVP adds a workspace page at `/workspace/scheduled-tasks` plus a background scheduler service gated by `config.yaml -> scheduler.enabled`.
|
||||
- Scheduled background runs are intentionally non-interactive: they execute through the normal run lifecycle, but the lead-agent toolset excludes `ask_clarification` when `context.non_interactive=true`. The key is honored only for internally-authenticated callers (the scheduler launch path); client-supplied `context.non_interactive` is dropped.
|
||||
- Scheduled background runs are intentionally non-interactive: the lead-agent toolset excludes `ask_clarification` when `context.non_interactive=true`. That key, `disable_clarification`, and `github_token` are honored only for internally-authenticated callers; client-supplied copies are dropped from both `body.context` and `body.config`.
|
||||
- Busy scheduled occurrences are persisted as `queued`; `launching` is a short lease-fenced claim, `running` remains the normal Gateway run lifecycle, and `scheduler.queue_timeout_seconds` bounds the durable wait. Do not reintroduce skip-on-overlap or count waiting rows against `max_concurrent_runs`.
|
||||
- Scheduled launches use `scheduler.recursion_limit` (default 1000, matching the web UI's `recursion_limit: 1000`, clamped by `max_recursion_limit`). The value is read at dispatch, so a YAML edit applies to the next scheduled run without a Gateway restart.
|
||||
|
||||
## Commands: Root vs. Module
|
||||
|
||||
|
||||
15
CHANGELOG.md
15
CHANGELOG.md
@ -558,6 +558,20 @@ This section accumulates work toward the **2.1.0** milestone
|
||||
|
||||
### Fixed
|
||||
|
||||
- **gateway:** Honor `disable_clarification` and `github_token` only for
|
||||
internally-authenticated callers, the way `non_interactive` already was.
|
||||
Both keys were forwarded from `body.context` regardless of the caller and
|
||||
were not scrubbed from the free-form `body.config` that the run config
|
||||
copies verbatim, so any session or PAT caller could set them.
|
||||
`disable_clarification` is the stronger of the two: `ClarificationMiddleware`
|
||||
answers every clarification — `risk_confirmation` included — with "proceed
|
||||
without asking", and `SandboxMiddleware` reads it as the same
|
||||
non-interactive signal as `non_interactive`. `github_token` reached
|
||||
`runtime.context`, where the bash tool exports it as `GH_TOKEN`/`GITHUB_TOKEN`,
|
||||
and a copy smuggled through `body.config['configurable']` was persisted in
|
||||
the checkpoint store. The scheduler, IM channels, and the GitHub webhook
|
||||
channel authenticate over the internal request channel and are unaffected.
|
||||
([#5338])
|
||||
- **artifacts:** Keep `PUT /api/threads/{id}/artifacts/{path}` confined to
|
||||
`/mnt/user-data/outputs`. The outputs-only guard was a string-prefix check on
|
||||
the raw path, so a percent-encoded `..` (`outputs/%2e%2e/uploads/x.txt`) —
|
||||
@ -2699,3 +2713,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#5284]: https://github.com/bytedance/deer-flow/pull/5284
|
||||
[#5287]: https://github.com/bytedance/deer-flow/pull/5287
|
||||
[#5321]: https://github.com/bytedance/deer-flow/pull/5321
|
||||
[#5338]: https://github.com/bytedance/deer-flow/pull/5338
|
||||
|
||||
@ -397,6 +397,16 @@
|
||||
|
||||
### 修复
|
||||
|
||||
- **Gateway：** `disable_clarification` 与 `github_token` 现在与 `non_interactive`
|
||||
一样，仅对内部认证的调用方生效。此前这两个键无论调用方身份都会从 `body.context`
|
||||
透传，而且不会从被逐字复制进 run config 的自由格式 `body.config` 中清除，因此任何
|
||||
会话或 PAT 调用方都能设置它们。其中 `disable_clarification` 影响更大：
|
||||
`ClarificationMiddleware` 会把包括 `risk_confirmation` 在内的所有澄清请求替换为
|
||||
"无需确认，继续执行"，`SandboxMiddleware` 也把它与 `non_interactive` 视作同一个
|
||||
非交互信号。`github_token` 则会进入 `runtime.context`，被 bash 工具导出为
|
||||
`GH_TOKEN`/`GITHUB_TOKEN`；若经由 `body.config['configurable']` 夹带，还会被写入
|
||||
checkpoint 存储。定时任务、IM 渠道与 GitHub webhook 渠道走内部请求通道，不受影响。
|
||||
([#5338])
|
||||
- **Artifact：** `PUT /api/threads/{id}/artifacts/{path}` 现在严格限制在
|
||||
`/mnt/user-data/outputs` 之内。此前 outputs-only 校验只是对原始路径做字符串前缀
|
||||
检查，百分号编码的 `..`（`outputs/%2e%2e/uploads/x.txt`，nginx 原样转发、Starlette
|
||||
@ -2091,3 +2101,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子
|
||||
[#5284]: https://github.com/bytedance/deer-flow/pull/5284
|
||||
[#5287]: https://github.com/bytedance/deer-flow/pull/5287
|
||||
[#5321]: https://github.com/bytedance/deer-flow/pull/5321
|
||||
[#5338]: https://github.com/bytedance/deer-flow/pull/5338
|
||||
|
||||
@ -15,7 +15,7 @@ DeerFlow is a LangGraph-based AI super agent system with a full-stack architectu
|
||||
- Gateway streams `write_file` and `str_replace` argument deltas in bounded batches for multi-mode `messages-tuple` consumers; single-mode message consumers retain the original per-chunk contract. Non-message frames flush pending batches, and `values` remains an optional complete-state snapshot rather than a prerequisite for batching.
|
||||
- With `stream_subgraphs`, subgraph frames keep their namespace in the SSE event name (`values|<ns>`, LangGraph Platform style) instead of impersonating root frames — a delegated subagent inherits the parent checkpoint namespace, so publishing its `values` snapshot as bare `values` replaces the whole thread view in SDK clients (#4399). Root-only consumers (file-tool chunk batcher, subagent event persistence, LLM error-fallback detection) ignore namespaced frames. The web frontend does not request subgraph streaming; subtask progress rides root-namespace `task_*` custom events.
|
||||
- Background subagent identity is deliberately split: the provider `tool_call_id` remains the correlation key for `ToolMessage`, `task_*` SSE events, persisted lifecycle events, frontend cards, and the public `ExtensionData.scope_id` contract (stored as `SubagentResult.external_task_id`), while `SubagentExecutor.execute_async()` generates a full server-side `execution_id` for `SubagentResult.task_id`, the process-wide registry, polling, cancellation, timeout handling, and cleanup. Provider IDs are not globally unique across parent runs, so they must never become registry ownership keys; scheduler closures retain their own `SubagentResult` rather than resolving ownership again through the mutable registry. Terminal subagent token usage travels in the current run's `ToolMessage.additional_kwargs` and is attributed from message state, never through a process-global provider-ID cache.
|
||||
- Scheduled-task executions must reuse that same Gateway run lifecycle. The scheduler may decide *when* work runs, but it must dispatch through the existing run path rather than introducing a parallel execution stack. Scheduled launches pass `scheduler.recursion_limit` (default 1000, matching the web UI's `recursion_limit: 1000`, clamped by `max_recursion_limit`) via `launch_scheduled_thread_run`; the value is read from `get_app_config()` at dispatch.
|
||||
- Scheduled-task executions must reuse that same Gateway run lifecycle. The scheduler may decide *when* work runs, but it must dispatch through the existing run path rather than introducing a parallel execution stack. Scheduled launches pass `scheduler.recursion_limit` (default 1000, matching the web UI's `recursion_limit: 1000`, clamped by `max_recursion_limit`) via `launch_scheduled_thread_run`; the value is read from `get_app_config()` at dispatch, so a YAML edit applies to the next scheduled run without a Gateway restart.
|
||||
- The background scheduler is single-instance by default. `scheduler.multi_instance=true` opts into lease-aware recovery across Gateway instances and requires shared Postgres, `run_ownership.heartbeat_enabled=true`, and `run_events.backend=db`; otherwise startup rejects the configuration. Live scheduled runs are preserved when a peer starts; expired launch claims return to the durable queue, expired run leases are atomically taken over, stale launch writes are fenced by lease ownership, and the Postgres advisory-locked budget makes `max_concurrent_runs` a shared global cap for `launching`/`running` rows.
|
||||
- Long-running MCP work uses a separate durable task runtime (`McpTaskService` + `mcp_tasks`, lease-based recovery) rather than keeping remote task IDs or status polling inside the Agent loop; only submit remains Agent-visible, the database is the source of truth, and `ThreadState` receives only a bounded current-thread projection. Full contract (leases, cancellation fencing, delivery idempotency, management-tool exposure): [packages/harness/deerflow/mcp/AGENTS.md](packages/harness/deerflow/mcp/AGENTS.md).
|
||||
- MCP task notification retries, dead-lettering, and the cancel endpoint's worker-stopped 503 are part of that same contract — see [packages/harness/deerflow/mcp/AGENTS.md](packages/harness/deerflow/mcp/AGENTS.md).
|
||||
@ -225,6 +225,15 @@ the tool graph or subagent executor during state/schema imports.
|
||||
SQLite, and PostgreSQL: missing differs from null, bool differs from int, and
|
||||
float filters accept integer or real JSON numbers through `json_value_matches`.
|
||||
|
||||
### Gateway Run-Context Trust Boundary
|
||||
|
||||
A server-produced run-context key must be gated on both client-writable feeds:
|
||||
`body.context` (whitelist-merged) and free-form `body.config` (copied verbatim).
|
||||
`merge_run_context_overrides` forwards it only when `internal=True`;
|
||||
`strip_internal_context_keys` scrubs it from the assembled `context` *and*
|
||||
`configurable`. Trust and destination are separate axes, so a new key needs both
|
||||
decisions — and `disable_clarification` is no milder than `non_interactive`.
|
||||
|
||||
## Development Workflow
|
||||
|
||||
### Test-Driven Development (TDD) — MANDATORY
|
||||
|
||||
@ -457,8 +457,28 @@ _SERVER_OWNED_RUNTIME_CONTEXT_KEYS: frozenset[str] = (
|
||||
# ``disable_clarification`` — set for non-interactive channels (GitHub
|
||||
# webhooks) so ClarificationMiddleware proceeds
|
||||
# instead of dead-ending the run.
|
||||
#
|
||||
# Both are produced server-side by the channel run policies
|
||||
# (``ChannelManager._apply_channel_policy`` and ``app.gateway.github.run_policy``),
|
||||
# which reach the Gateway over the internally-authenticated request channel, so
|
||||
# they are internal-only as well — see :data:`_INTERNAL_ONLY_CONTEXT_KEYS`.
|
||||
_CONTEXT_RUNTIME_ONLY_KEYS: frozenset[str] = frozenset({"github_token", "disable_clarification"})
|
||||
|
||||
# Every run-context key an external client may never supply, in either section.
|
||||
# The two sets differ only in *where* a legitimate internal caller's value lands
|
||||
# (both sections vs. ``context`` alone); their trust requirement is identical.
|
||||
#
|
||||
# ``disable_clarification`` is not a milder cousin of ``non_interactive``:
|
||||
# ``ClarificationMiddleware`` answers every clarification — ``risk_confirmation``
|
||||
# included — with "proceed without asking" instead of interrupting, and
|
||||
# ``SandboxMiddleware`` reads the two keys as the same non-interactive signal.
|
||||
# Accepting it from a client therefore reproduces the effect the
|
||||
# ``non_interactive`` gate exists to prevent. ``github_token`` is a live
|
||||
# credential that ``bash`` exports as ``GH_TOKEN``/``GITHUB_TOKEN``, and a copy
|
||||
# smuggled through ``body.config['configurable']`` would be written to the
|
||||
# checkpoint store.
|
||||
_INTERNAL_ONLY_CONTEXT_KEYS: frozenset[str] = _CONTEXT_INTERNAL_CALLER_KEYS | _CONTEXT_RUNTIME_ONLY_KEYS
|
||||
|
||||
|
||||
def strip_internal_context_keys(config: dict[str, Any]) -> None:
|
||||
"""Drop internal-only keys a non-internal caller smuggled into the run config.
|
||||
@ -471,7 +491,7 @@ def strip_internal_context_keys(config: dict[str, Any]) -> None:
|
||||
for section in ("context", "configurable"):
|
||||
value = config.get(section)
|
||||
if isinstance(value, dict):
|
||||
for key in _CONTEXT_INTERNAL_CALLER_KEYS:
|
||||
for key in _INTERNAL_ONLY_CONTEXT_KEYS:
|
||||
value.pop(key, None)
|
||||
|
||||
|
||||
@ -492,10 +512,11 @@ def merge_run_context_overrides(config: dict[str, Any], context: Mapping[str, An
|
||||
by :func:`strip_internal_context_keys`.
|
||||
|
||||
A second set of keys (``_CONTEXT_RUNTIME_ONLY_KEYS`` — e.g. ``github_token``,
|
||||
``disable_clarification``) is forwarded into ``config['context']`` only, never
|
||||
``configurable``. These are secrets / runtime flags read by tools and middlewares
|
||||
from ``runtime.context``; keeping them out of ``configurable`` avoids persisting a
|
||||
short-lived token in the checkpoint store.
|
||||
``disable_clarification``) is likewise forwarded only when ``internal`` is True,
|
||||
and then into ``config['context']`` only, never ``configurable``. These are
|
||||
secrets / runtime flags read by tools and middlewares from ``runtime.context``;
|
||||
keeping them out of ``configurable`` avoids persisting a short-lived token in the
|
||||
checkpoint store.
|
||||
"""
|
||||
if not context:
|
||||
return
|
||||
@ -509,10 +530,12 @@ def merge_run_context_overrides(config: dict[str, Any], context: Mapping[str, An
|
||||
if isinstance(runtime_context, dict):
|
||||
runtime_context.setdefault(key, context[key])
|
||||
# Context-only keys (secrets / runtime flags) land in ``config['context']``
|
||||
# only — never ``configurable`` (which is persisted in checkpoints).
|
||||
for key in _CONTEXT_RUNTIME_ONLY_KEYS:
|
||||
if key in context and isinstance(runtime_context, dict):
|
||||
runtime_context.setdefault(key, context[key])
|
||||
# only — never ``configurable`` (which is persisted in checkpoints) — and only
|
||||
# for internal callers, the sole legitimate producers.
|
||||
if internal:
|
||||
for key in _CONTEXT_RUNTIME_ONLY_KEYS:
|
||||
if key in context and isinstance(runtime_context, dict):
|
||||
runtime_context.setdefault(key, context[key])
|
||||
if "user_id" in context and isinstance(runtime_context, dict):
|
||||
runtime_context.setdefault("user_id", context["user_id"])
|
||||
|
||||
|
||||
@ -1673,6 +1673,7 @@ def test_merge_run_context_overrides_forwards_context_only_keys():
|
||||
"disable_clarification": True,
|
||||
"agent_name": "coding-llm-gateway",
|
||||
},
|
||||
internal=True,
|
||||
)
|
||||
|
||||
# Forwarded into runtime context — what tools/middlewares read.
|
||||
@ -1685,15 +1686,46 @@ def test_merge_run_context_overrides_forwards_context_only_keys():
|
||||
assert "disable_clarification" not in config.get("configurable", {})
|
||||
|
||||
|
||||
def test_context_only_keys_are_internal_only():
|
||||
"""``github_token`` / ``disable_clarification`` are produced by the channel run
|
||||
policies, which reach the Gateway over the internally-authenticated channel. A
|
||||
non-internal caller must not be able to supply either through ``body.context``.
|
||||
|
||||
``disable_clarification`` is not a milder cousin of ``non_interactive``:
|
||||
``ClarificationMiddleware`` answers every clarification — ``risk_confirmation``
|
||||
included — with "proceed without asking", and ``SandboxMiddleware`` reads the two
|
||||
keys as the same non-interactive signal. Forwarding it ungated reopened exactly
|
||||
the gate ``_CONTEXT_INTERNAL_CALLER_KEYS`` exists to close.
|
||||
"""
|
||||
from app.gateway.services import build_run_config, merge_run_context_overrides
|
||||
|
||||
config = build_run_config("thread-1", None, None)
|
||||
merge_run_context_overrides(
|
||||
config,
|
||||
{
|
||||
"github_token": "attacker-supplied",
|
||||
"disable_clarification": True,
|
||||
"agent_name": "coding-llm-gateway",
|
||||
},
|
||||
)
|
||||
|
||||
assert "github_token" not in config["context"]
|
||||
assert "disable_clarification" not in config["context"]
|
||||
assert "github_token" not in config.get("configurable", {})
|
||||
assert "disable_clarification" not in config.get("configurable", {})
|
||||
# Whitelisted agent-config keys still come through for ordinary callers.
|
||||
assert config["context"]["agent_name"] == "coding-llm-gateway"
|
||||
|
||||
|
||||
def test_merge_run_context_overrides_context_only_keys_do_not_override_existing():
|
||||
"""A token already in ``config['context']`` must not be clobbered by a
|
||||
client-supplied one (defense in depth — the manager is the only legitimate
|
||||
source, but ``setdefault`` keeps the contract explicit)."""
|
||||
"""A token already in ``config['context']`` must not be clobbered by one supplied
|
||||
in ``body.context`` (defense in depth — ``setdefault`` keeps the contract explicit
|
||||
even now that only internal callers reach this branch)."""
|
||||
from app.gateway.services import build_run_config, merge_run_context_overrides
|
||||
|
||||
config = build_run_config("thread-1", None, None)
|
||||
config["context"] = {"github_token": "pre-existing"}
|
||||
merge_run_context_overrides(config, {"github_token": "attacker-supplied"})
|
||||
merge_run_context_overrides(config, {"github_token": "later-supplied"}, internal=True)
|
||||
|
||||
assert config["context"]["github_token"] == "pre-existing"
|
||||
|
||||
@ -2916,6 +2948,71 @@ def test_strip_internal_context_keys_scrubs_config_smuggled_non_interactive():
|
||||
assert "non_interactive" not in via_configurable["configurable"]
|
||||
|
||||
|
||||
def test_strip_internal_context_keys_scrubs_config_smuggled_context_only_keys():
|
||||
"""The context-only internal keys need the same ``body.config`` scrub as
|
||||
``non_interactive``: ``build_run_config`` copies both sections verbatim, so gating
|
||||
``merge_run_context_overrides`` alone still leaves ``body.config['context']`` open.
|
||||
|
||||
The ``configurable`` half matters on its own — that dict is persisted in
|
||||
checkpoints, so a smuggled ``github_token`` would write a live credential into the
|
||||
checkpoint store even though no tool reads it from there.
|
||||
"""
|
||||
from app.gateway.services import build_run_config, strip_internal_context_keys
|
||||
|
||||
via_context = build_run_config(
|
||||
"thread-1",
|
||||
{"context": {"github_token": "attacker-supplied", "disable_clarification": True, "model_name": "gpt"}},
|
||||
None,
|
||||
)
|
||||
strip_internal_context_keys(via_context)
|
||||
assert "github_token" not in via_context["context"]
|
||||
assert "disable_clarification" not in via_context["context"]
|
||||
assert via_context["context"]["model_name"] == "gpt"
|
||||
|
||||
via_configurable = build_run_config(
|
||||
"thread-1",
|
||||
{"configurable": {"github_token": "attacker-supplied", "disable_clarification": True}},
|
||||
None,
|
||||
)
|
||||
strip_internal_context_keys(via_configurable)
|
||||
assert "github_token" not in via_configurable["configurable"]
|
||||
assert "disable_clarification" not in via_configurable["configurable"]
|
||||
|
||||
|
||||
def test_start_run_sequence_drops_context_only_keys_for_session_caller():
|
||||
"""Replay the real ``start_run`` assembly order for a session-authenticated caller
|
||||
that pushes the keys through *both* smuggling surfaces at once."""
|
||||
request = _make_request_with_auth_source("session")
|
||||
config = _assemble_authz_run_config(
|
||||
{"context": {"github_token": "via-config", "disable_clarification": True}},
|
||||
request,
|
||||
body_context={"github_token": "via-body-context", "disable_clarification": True},
|
||||
)
|
||||
|
||||
assert "github_token" not in config["context"]
|
||||
assert "disable_clarification" not in config["context"]
|
||||
assert "github_token" not in config.get("configurable", {})
|
||||
assert "disable_clarification" not in config.get("configurable", {})
|
||||
|
||||
|
||||
def test_start_run_sequence_keeps_context_only_keys_for_internal_caller():
|
||||
"""The channel path (internal auth) must keep carrying the minted token and the
|
||||
non-interactive flag, and neither may land in checkpoint-persisted ``configurable``."""
|
||||
from app.gateway.internal_auth import INTERNAL_SYSTEM_ROLE
|
||||
|
||||
request = _make_request_with_auth_source(AUTH_SOURCE_INTERNAL, system_role=INTERNAL_SYSTEM_ROLE)
|
||||
config = _assemble_authz_run_config(
|
||||
{},
|
||||
request,
|
||||
body_context={"github_token": "ghs_installation_token", "disable_clarification": True},
|
||||
)
|
||||
|
||||
assert config["context"]["github_token"] == "ghs_installation_token"
|
||||
assert config["context"]["disable_clarification"] is True
|
||||
assert "github_token" not in config.get("configurable", {})
|
||||
assert "disable_clarification" not in config.get("configurable", {})
|
||||
|
||||
|
||||
# --- Authorization identity anti-forgery tests ---
|
||||
|
||||
|
||||
|
||||
@ -154,7 +154,7 @@ These span both layers and require reading multiple files to understand:
|
||||
tasks reuse the *same* Gateway run lifecycle (scheduler decides *when*, not *how*).
|
||||
- **Scheduled tasks** — workspace page `/workspace/scheduled-tasks` + a background scheduler
|
||||
gated by `config.yaml → scheduler.enabled`; non-interactive runs drop `ask_clarification`
|
||||
and client-supplied `non_interactive`.
|
||||
and client-supplied `non_interactive` (see the run-context trust boundary in §6).
|
||||
- **Long-running MCP** — a durable `McpTaskService` (leased rows, DB as source of truth)
|
||||
keeps remote task IDs/polling out of the agent loop.
|
||||
- **Version sources** — a release version must match in `backend/pyproject.toml`,
|
||||
@ -172,6 +172,12 @@ These span both layers and require reading multiple files to understand:
|
||||
sandbox is dev-only direct execution.
|
||||
- **MCP isolation**: each MCP server runs in its own process with runtime env-var
|
||||
resolution; servers toggle independently.
|
||||
- **Run-context trust boundary**: run context reaches the agent from two client-writable
|
||||
surfaces — `body.context` and the free-form `body.config` — so every server-produced key
|
||||
is gated on both. `non_interactive`, `disable_clarification`, and `github_token` are
|
||||
honored only for internally-authenticated callers (the scheduler and IM/webhook channel
|
||||
policies) and scrubbed from a non-internal caller's config; identity and sandbox
|
||||
lifecycle fields are cleared unconditionally and restamped from auth state.
|
||||
- **Loopback-by-default ingress**: nginx is the only published surface; the Gateway's `8001`
|
||||
is container-internal and never published. A bare `"${PORT}:2026"` bind (0.0.0.0) is
|
||||
rejected by convention and CI. See the Security Notice in [`README.md`](../README.md) before
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user