fix(code): resolve project policy per workspace - #6064
Open
Mason Daugherty (mdrxy) wants to merge 29 commits into
Open
fix(code): resolve project policy per workspace#6064Mason Daugherty (mdrxy) wants to merge 29 commits into
Mason Daugherty (mdrxy) wants to merge 29 commits into
Conversation
github-actions
Bot
added
dcode
Mason Daugherty (mdrxy)
marked this pull request as ready for review
September 3, 2026 21:30
![open-swe[bot]](https://avatars.githubusercontent.com/in/1395372?s=60&v=4){kind=small}
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
`_dotenv_values_from` builds `DotEnv` directly, and its `encoding` defaults to `None` -- the locale encoding -- where the `dotenv_values` helper it replaced defaulted to UTF-8. On a non-UTF-8 locale a `.env` holding non-ASCII bytes raises `UnicodeDecodeError`, which is a `ValueError` that `_dotenv_environment` swallows, so the whole file is dropped with only a warning. Pass `encoding="utf-8"` explicitly.
`resolve_read_project_dotenv` defaults to true, so an `OSError` reading `~/.deepagents/.env` left `global_toggle` empty and silently discarded the user's `startup.read_project_dotenv` opt-out, loading the untrusted project `.env` instead. The only signal was a warning that did not mention the toggle. `_dotenv_environment` now runs once per workspace runtime rather than once per process, so the fail-open window is re-entered on every new workspace. Fail closed instead, and say so in the warning. The synthetic value occupies the global-dotenv tier only, so managed policy and a shell export still win.
`apply_dotenv` mutates `env` in place and the project file is applied
first, so the trusted `~/.deepagents/.env` resolved its `${VAR}`
references against values a cloned repo supplied. A global
`ANTHROPIC_BASE_URL=${GATEWAY_HOST}/v1` would follow a `GATEWAY_HOST`
planted by a repo's committed `.env`, with no log line -- the value
simply resolved differently.
Parsing each file with `dotenv_values()` previously interpolated against
`os.environ` alone, so this arrived with the workspace rewrite.
Snapshot the environment before the project layer and interpolate the
global file against that. Trusted shell values stay visible; project
values no longer are.
`_resolve_sdk_kwargs` gated on a `DEEPAGENTS_CODE_`-prefixed name being
present, then returned `{}` so the Vercel SDK could resolve credentials
from `os.environ` itself. That fallback no longer reaches the workspace:
`_build_server_env` strips the client's project `.env` from the server
process, and each workspace re-resolves its own snapshot. A workspace
`.env` carrying the canonical `VERCEL_TOKEN` was therefore discarded in
silence, and the sandbox authenticated as the server process or failed
with an opaque Vercel auth error.
Gate on the resolved values instead, matching every other provider,
which resolves through `resolve_env_var` and raises when nothing is
found. Delegation to the SDK now happens only when the workspace
configured no credential at all.
Two reload tests pinned the behavior removed in 54f0259: an unreadable `~/.deepagents/.env` used to leave `read_project_dotenv` at its `True` default, so the project `.env` loaded anyway. They now assert the opt-out survives the failure, and match the new warning text. The client reload path shares `_dotenv_environment`, so it fails closed alongside the server.
…ials Three problems in the workspace AWS path: `_aws_session_kwargs` resolved through a raw `Mapping.get` while the model path fed the same `AWS_CREDENTIAL_ENV_SOURCES` table through `resolve_env_var`. A `DEEPAGENTS_CODE_AWS_*` override therefore applied to the model and was dropped for the sandbox. Sharing the table alone left the lookup free to drift. `resolve_env_kwargs` omits each argument independently, so a workspace `.env` with `AWS_ACCESS_KEY_ID` but no `AWS_SECRET_ACCESS_KEY` built a session boto3 rejects with `PartialCredentialsError`. That error, like every other, was swallowed into `session = None`, which makes the SDK resolve credentials from the server process. A workspace scoped to a restricted profile then ran its sandbox under the server's broader identity, and the only signal was a background warning. Resolve through `resolve_env_var`, reject a half-set access-key pair by name, and raise instead of falling back when the workspace scoped credentials at all. Falling back stays correct when it scoped none.
`apply_inherited_user_tracing` wrote every key the relayed JSON carried straight into the `execute` shell environment. The carrier is denied from every `.env` and popped before export, so the trust boundary held, but this is the one place a serialized blob becomes shell environment and the keys were trusted rather than constrained. The function's stated contract -- mirroring `restore_user_tracing_env` / `restore_user_tracing_api_keys` -- was narrower than what it did. Constrain writes to the tracing flags and keys the fail-closed branch already enumerates, and warn about anything else. Also add `exc_info` to the parse warning, which gave no clue what was malformed.
`apply_inherited_user_tracing` writes the caller's real LangSmith key into `shell_env`, so it is the credential `execute` commands run under. `_known_credential_values` builds the redaction set by matching env var *names* against `_SECRET_KEY_RE`, and the key reaches this process only inside the `DEEPAGENTS_INHERITED_USER_TRACING` carrier, whose name does not match -- and whose value is a JSON blob, not the key. The name scan registered `LANGSMITH_API_KEY` from the process environment instead, which holds the *agent's* key. The one credential user commands actually use was therefore the one credential Auto mode would not redact from command output or classifier reasons. Add `relayed_user_tracing_secrets` and fold its values into the redaction set.
`trust_project_extensions` is the only project policy field not derived from the environment -- `resolve_workspace` re-reads it from the persisted trust store for a project other than the launch project. The existing drift test flips `trust_project_mcp` through the environment, which also moves the server-config fingerprint, so it never exercised the project-policy comparison alone. This pins the case that comparison exists for: the user revokes trust while `ServerConfig.from_env()` is unchanged, so the fingerprint check cannot fire. Deleting the comparison in `_resolve_bound_workspace_config` fails this test, and a runtime would keep executing project Python the user has untrusted.
Three `except RuntimeError` branches exist only to keep a resolved API
key from reaching an endpoint it was not issued for when the credential
file is unreadable. `TestWorkspaceStoredCredentials` covered every happy
path and the key/endpoint pairing, but none of the failure branches:
deleting `kwargs.pop("base_url", None)` from either endpoint guard left
the suite green while sending a key to an `OPENAI_BASE_URL` a workspace
`.env` chose.
Each new test fails when its guard is removed. The Vertex case also pins
the warning, since that provider uses implicit auth and would otherwise
surface a corrupt store only as an opaque ADC error.
…graphs Every other `_make_graphs` test injects a fake `deepagents_code.config` whose `use_environment` is `nullcontext`, and each consumer test patches `active_environment` directly. Both ends were mocked and the wire between them was never exercised: replacing `use_environment(workspace_env)` with `use_environment(None)` left the whole suite green while every consumer silently fell back to `os.environ`. That is the failure the sandbox comments warn about -- `_run_sandbox_setup` would expand the server process's secrets into the setup script instead of the workspace's. Run `_make_graphs` against the real config module with a real workspace `.env`, and assert `create_model`, `create_cli_agent` and `create_sandbox` each read the workspace value at call time, while the process environment stays clean.
Quality cleanup over the workspace-scoped project config work. No intended
behavior change.
Duplication:
- `_server_config._fingerprint` was a byte-for-byte copy of the fingerprint
wire format in `workspace.py`, which client claims and server verification
must agree on. Promoted to `workspace.canonical_fingerprint()`.
- Project-policy drift was detected two ways, with two copies of the same two
user-facing reason strings. Now `workspace.project_policy_differs()` plus
shared reason constants.
- The dotenv "un-inject what this loader injected" loop lived in three places,
one of which reached across a package boundary for `_dotenv_loaded_values`.
Extracted `config.strip_loaded_dotenv_values()`.
- `to_session_workspace_claim` / `to_project_workspace_policy` were identical
comprehensions over different field sets.
Altitude:
- Dropped the `_workspace_web_search_tools` weakref registry and
`is_web_search_tool`. `_build_tools` now returns the read-only built-ins it
created, so `_criteria_context_tools` no longer re-derives read-only-ness by
object identity from another module.
- `workspace_claim_fields()` / `project_workspace_fields()` were zero-arg
accessors around two frozensets; the field sets are now public constants.
Simplification:
- `_build_tools`' `has_tavily` + `tavily_api_key` pair let an inconsistent
caller build a search tool with an empty key; replaced by one credentials
snapshot.
- `create_model`: hoisted `scoped_environment` / `stored_credential` so the
later `provider and ...` guards stop being UnboundLocalError dodges, and
extracted the five-deep scoped-endpoint block into `_apply_scoped_endpoint`.
- Removed the `/workspace` claim-key gate that is strictly implied by the dict
comparison below it, which returns the identical 409 and detail.
Efficiency:
- `_workspace_runtime` ran full policy revalidation twice per runtime build
(`ServerConfig.from_env()`, two strict path resolves, an uncached trust-store
read, two fingerprints) and discarded the first result.
- The launch binding and the `/workspace` route each made three sequential
`to_thread` hops for one strictly-dependent blocking unit.
- `mcp_config._interpolate_env` resolved the active environment inside the
regex callback, once per `${VAR}` match.
- `create_cli_agent` and `_known_credential_values` resolved `environ=None`
against `os.environ` while every other reader used `active_environment()`,
so inside a bound scope they read the process environment.
`_apply_azure_sdk_endpoint` wrote `azure_endpoint` unconditionally when `AZURE_OPENAI_ENDPOINT` was set, so an explicit `azure_endpoint` resolved from `config.toml` params was silently overwritten and requests and credentials were routed to the environment endpoint instead. Restore `setdefault` so the environment only fills in a missing endpoint, matching the other SDK environment defaults in `_apply_provider_sdk_environment`.
`_resolve_sdk_kwargs` warned about a missing `VERCEL_TOKEN` /
`VERCEL_PROJECT_ID` / `VERCEL_TEAM_ID` field and returned `{}`, handing
auth back to the Vercel SDK. The SDK resolves credentials from the server
process, whose environment no longer carries the workspace `.env` (it is a
bound snapshot `_build_server_env` strips from the process). A workspace
that pinned a restricted token therefore silently ran its sandbox under the
server's ambient or OIDC identity -- the same privilege substitution the
AgentCore change rejects.
Raise `ValueError` instead, so `create_sandbox` surfaces a startup error
naming the missing fields and how to fix them. Delegation to the SDK stays
correct when the workspace resolves none of the three variables.
Mason Daugherty (mdrxy)
force-pushed
the
mdrxy/code/workspace-project-config
branch
from
September 8, 2026 03:30
f311351 to
e56921c
Compare
A half-set MODAL_TOKEN_ID / MODAL_TOKEN_SECRET pair left the client as `None`, so `App.lookup` resolved credentials from the server process. A workspace that pinned a restricted token silently ran its sandbox under the server's broader Modal identity, and the warning that said so has no default reader. Raise instead, matching the AgentCore and Vercel providers.
`credential_kwargs` being non-empty was read as "this workspace pinned credentials", but it resolves through `active_environment()`, which falls back to `os.environ`. A plain `AWS_PROFILE` exported in the server's own shell therefore counted as workspace-pinned, so a stale profile became a startup failure whose message blamed a workspace `.env` that never set it. Compare the resolved arguments against what the server resolves on its own and fail closed only on the difference. Also reject a session token with neither access-key half: botocore declines the explicit credential provider and falls through to the server's credentials, so the workspace's token was silently ignored. `AWS_PROFILE` alongside explicit keys is left alone -- boto3's precedence is deterministic and the combination arises normally once the workspace layers over the server environment.
Gating on `any(values.values())` read the server's own `VERCEL_*` as workspace-pinned, because `resolve_env_var` falls back to `os.environ`. A personal-scope account -- token plus project id, no team id, which Vercel allows -- therefore hard-failed at startup where it previously worked. Delegate to the SDK whenever every resolved value matches the server environment: there is no workspace identity to protect, and the SDK resolves exactly those values. A workspace override still differs from the server and takes the fail-closed path. This subsumes the inherited-OIDC carve-out, which was the same check restricted to token-free sets.
…licy Resolving project policy per workspace changed which fields the binding fingerprint spans, but the schema version stayed at 2. Threads already bound outside the launch project were bound with the launch config's fingerprint, so every request refused with SERVER_CONFIG_DRIFT_REASON -- and `_bind` declines to overwrite a row that already has a fingerprint, so re-binding raised the same conflict. Those threads had no recovery path. Bump the schema to 3 and treat a stale row's fingerprint as uncomparable rather than as drift: `_bind` migrates it in place. Workspace identity is still compared, so a migration only ever rewrites policy for the same directory, and a current-schema row still conflicts on real drift.
`trust_project_extensions` is resolved from a mutable on-disk store and feeds the drift check, which now runs before the runtime cache lookup. Granting trust for a directory in any other session therefore made every request on an already-bound thread refuse, permanently. A transient trust-store read failure did the same, because the trust store fails closed to an empty store. A grant is user-authorized and only ever adds privilege, so pin the bound value rather than refusing: the thread keeps the trust it was bound with and the grant applies to the next binding. Revocation still refuses immediately. Name the drifted fields in the refusal and log them. The values are paths and booleans, never secrets, and without them a trust-store read failure is indistinguishable from a real policy change.
`_default_workspace_binding` resolved project policy against the root `find_project_root` derives, but `_get_runtime()` takes no config override, so the process-wide runtime builds from `ServerConfig.from_env()` and `get_server_project_context()` -- which prefers an explicit `DEEPAGENTS_CODE_SERVER_PROJECT_ROOT`. Where the two disagreed, the binding recorded scrubbed project policy while the runtime kept the launch project's MCP servers and extensions live: the cache was keyed by a config that was never built from. Resolve against the explicit root when it is set, so the binding and the runtime describe the same policy.
`ServerConfig.resolve_workspace` said it returns "a config bound to the target workspace's project policy". For any directory other than the launch project it *discards* that policy -- a reader would expect the target's `.mcp.json` and `sandbox_setup` to be picked up. State what it does, and why the two branches exist. Also: - Document `SESSION_WORKSPACE_FIELDS` and `PROJECT_WORKSPACE_FIELDS`, including the partition invariant the trust boundary depends on. - Log `_same_workspace_project`'s fail-closed decision. Returning `False` on an unresolvable path silently drops the user's `--sandbox-setup` and `--mcp-config`, with no message anywhere. - Drop the claim that `canonical_fingerprint` is the single definition of the wire format; `canonical_workspace_config` repeats the same encoding. - `workspace.resolve_workspace` no longer takes a client policy claim; only `cwd` is client-supplied. Say so, so nobody hands it `body["workspace_config"]`. - The AWS `Raises:` rationale described the silent fallback this PR removed.
Three gaps let the PR's headline behavior regress silently: - Nothing asserted that `_make_graphs` receives the scrubbed policy. The resolution was tested in isolation and the drift check was tested, but returning the unresolved config, or losing one of the five strips, kept every test green. Mutation-checked: replacing the returned config fails the new test. - The offload bind test computed its expected payload by calling `resolve_workspace` -- the method under test -- so it passed even if the stripping stopped entirely. It also ran with `cwd=None`, taking the drop branch with nothing to drop. Assert the literal policy from a real launch config instead. - `read_only_builtins` had no coverage: both `_criteria_context_tools` tests are handed the allowlist as an argument, and the `is_web_search_tool` it replaced had no test. `read_only_builtins = tools` would have exposed mutating built-ins to rubric grading silently.
Share one canonical JSON encoding between `canonical_workspace_config` and `canonical_fingerprint`, so the wire format client claims and server verification agree on has a single definition. Compare project directories with `_paths._same_directory`, which compares by device and inode. The previous `resolve() ==` missed a symlinked or differently cased spelling of one directory on the case-insensitive filesystems that are the default on macOS and Windows. Generalize the AWS server-environment baseline into `_server_env_kwargs`, and drive the Vercel credentials off one `_CREDENTIAL_ENV_NAMES` table rather than deriving env var names from SDK argument names in three places. A renamed argument can no longer desynchronize the error message from what was read. Decode the relayed tracing carrier in one place, promote the workspace request allowlist to a module constant beside the other request-shape constants, and drop the single-use `project_policy_differs` and `_workspace_subset` wrappers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each server workspace now uses the project policy for its own directory instead of inheriting policy from the directory that launched the server.
flowchart TD S["Shared server policy: approvals, capabilities, limits"] --> A["Workspace A policy"] S --> B["Workspace B policy"] PA["Project A settings and trust"] --> A PB["Project B settings and trust"] --> BMade by Open SWE