2026-02-10 11:22:52 -08:00
|
|
|
"""Shared sandbox state used by server and middleware."""
|
|
|
|
|
|
|
|
|
|
from __future__ import annotations
|
|
|
|
|
|
2026-02-17 15:03:20 -08:00
|
|
|
import asyncio
|
|
|
|
|
import logging
|
2026-02-10 11:22:52 -08:00
|
|
|
|
2026-05-11 16:03:38 -07:00
|
|
|
from deepagents.backends.protocol import (
|
|
|
|
|
EditResult,
|
|
|
|
|
ExecuteResponse,
|
|
|
|
|
FileDownloadResponse,
|
|
|
|
|
FileUploadResponse,
|
|
|
|
|
GlobResult,
|
|
|
|
|
GrepResult,
|
|
|
|
|
LsResult,
|
|
|
|
|
ReadResult,
|
|
|
|
|
SandboxBackendProtocol,
|
|
|
|
|
WriteResult,
|
|
|
|
|
)
|
2026-02-17 16:50:08 -08:00
|
|
|
from langgraph.config import get_config
|
2026-02-17 15:03:20 -08:00
|
|
|
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
from .github_token import repo_cache_key
|
2026-03-17 11:55:36 -07:00
|
|
|
from .sandbox import create_sandbox
|
2026-02-17 15:03:20 -08:00
|
|
|
|
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
|
|
2026-05-11 16:03:38 -07:00
|
|
|
|
|
|
|
|
class SandboxBackendProxy(SandboxBackendProtocol):
|
|
|
|
|
"""Stable per-thread backend handle whose target can be replaced."""
|
|
|
|
|
|
|
|
|
|
def __init__(self, backend: SandboxBackendProtocol) -> None:
|
|
|
|
|
self._backend = backend
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
# "owner/name" of the repo this sandbox is bound to, used to refuse
|
|
|
|
|
# reuse by a different repo presenting a colliding thread_id.
|
|
|
|
|
self.bound_repo: str | None = None
|
2026-05-11 16:03:38 -07:00
|
|
|
|
|
|
|
|
@property
|
|
|
|
|
def current(self) -> SandboxBackendProtocol:
|
|
|
|
|
return self._backend
|
|
|
|
|
|
|
|
|
|
@property
|
|
|
|
|
def id(self) -> str:
|
|
|
|
|
return self._backend.id
|
|
|
|
|
|
|
|
|
|
def replace_backend(self, backend: SandboxBackendProtocol) -> None:
|
|
|
|
|
self._backend = backend
|
|
|
|
|
|
|
|
|
|
def ls(self, path: str) -> LsResult:
|
|
|
|
|
return self._backend.ls(path)
|
|
|
|
|
|
|
|
|
|
async def als(self, path: str) -> LsResult:
|
|
|
|
|
return await self._backend.als(path)
|
|
|
|
|
|
|
|
|
|
def read(self, file_path: str, offset: int = 0, limit: int = 2000) -> ReadResult:
|
|
|
|
|
return self._backend.read(file_path, offset, limit)
|
|
|
|
|
|
|
|
|
|
async def aread(self, file_path: str, offset: int = 0, limit: int = 2000) -> ReadResult:
|
|
|
|
|
return await self._backend.aread(file_path, offset, limit)
|
|
|
|
|
|
|
|
|
|
def grep(
|
|
|
|
|
self,
|
|
|
|
|
pattern: str,
|
|
|
|
|
path: str | None = None,
|
|
|
|
|
glob: str | None = None,
|
|
|
|
|
) -> GrepResult:
|
|
|
|
|
return self._backend.grep(pattern, path, glob)
|
|
|
|
|
|
|
|
|
|
async def agrep(
|
|
|
|
|
self,
|
|
|
|
|
pattern: str,
|
|
|
|
|
path: str | None = None,
|
|
|
|
|
glob: str | None = None,
|
|
|
|
|
) -> GrepResult:
|
|
|
|
|
return await self._backend.agrep(pattern, path, glob)
|
|
|
|
|
|
|
|
|
|
def glob(self, pattern: str, path: str = "/") -> GlobResult:
|
|
|
|
|
return self._backend.glob(pattern, path)
|
|
|
|
|
|
|
|
|
|
async def aglob(self, pattern: str, path: str = "/") -> GlobResult:
|
|
|
|
|
return await self._backend.aglob(pattern, path)
|
|
|
|
|
|
|
|
|
|
def write(self, file_path: str, content: str) -> WriteResult:
|
|
|
|
|
return self._backend.write(file_path, content)
|
|
|
|
|
|
|
|
|
|
async def awrite(self, file_path: str, content: str) -> WriteResult:
|
|
|
|
|
return await self._backend.awrite(file_path, content)
|
|
|
|
|
|
|
|
|
|
def edit(
|
|
|
|
|
self,
|
|
|
|
|
file_path: str,
|
|
|
|
|
old_string: str,
|
|
|
|
|
new_string: str,
|
|
|
|
|
replace_all: bool = False,
|
|
|
|
|
) -> EditResult:
|
|
|
|
|
return self._backend.edit(file_path, old_string, new_string, replace_all)
|
|
|
|
|
|
|
|
|
|
async def aedit(
|
|
|
|
|
self,
|
|
|
|
|
file_path: str,
|
|
|
|
|
old_string: str,
|
|
|
|
|
new_string: str,
|
|
|
|
|
replace_all: bool = False,
|
|
|
|
|
) -> EditResult:
|
|
|
|
|
return await self._backend.aedit(file_path, old_string, new_string, replace_all)
|
|
|
|
|
|
|
|
|
|
def upload_files(self, files: list[tuple[str, bytes]]) -> list[FileUploadResponse]:
|
|
|
|
|
return self._backend.upload_files(files)
|
|
|
|
|
|
|
|
|
|
async def aupload_files(self, files: list[tuple[str, bytes]]) -> list[FileUploadResponse]:
|
|
|
|
|
return await self._backend.aupload_files(files)
|
|
|
|
|
|
|
|
|
|
def download_files(self, paths: list[str]) -> list[FileDownloadResponse]:
|
|
|
|
|
return self._backend.download_files(paths)
|
|
|
|
|
|
|
|
|
|
async def adownload_files(self, paths: list[str]) -> list[FileDownloadResponse]:
|
|
|
|
|
return await self._backend.adownload_files(paths)
|
|
|
|
|
|
|
|
|
|
def execute(self, command: str, *, timeout: int | None = None) -> ExecuteResponse:
|
|
|
|
|
return self._backend.execute(command, timeout=timeout)
|
|
|
|
|
|
|
|
|
|
async def aexecute(self, command: str, *, timeout: int | None = None) -> ExecuteResponse:
|
|
|
|
|
return await self._backend.aexecute(command, timeout=timeout)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# Thread ID -> stable SandboxBackendProxy, shared between server.py and middleware.
|
|
|
|
|
SANDBOX_BACKENDS: dict[str, SandboxBackendProxy] = {}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def unwrap_sandbox_backend(sandbox_backend: SandboxBackendProtocol) -> SandboxBackendProtocol:
|
|
|
|
|
if isinstance(sandbox_backend, SandboxBackendProxy):
|
|
|
|
|
return sandbox_backend.current
|
|
|
|
|
return sandbox_backend
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def set_sandbox_backend(
|
|
|
|
|
thread_id: str,
|
|
|
|
|
sandbox_backend: SandboxBackendProtocol,
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
*,
|
|
|
|
|
repo: str | None = None,
|
2026-05-11 16:03:38 -07:00
|
|
|
) -> SandboxBackendProxy:
|
|
|
|
|
if isinstance(sandbox_backend, SandboxBackendProxy):
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
if repo:
|
|
|
|
|
sandbox_backend.bound_repo = repo
|
2026-05-11 16:03:38 -07:00
|
|
|
SANDBOX_BACKENDS[thread_id] = sandbox_backend
|
|
|
|
|
return sandbox_backend
|
|
|
|
|
|
|
|
|
|
existing = SANDBOX_BACKENDS.get(thread_id)
|
|
|
|
|
if isinstance(existing, SandboxBackendProxy):
|
|
|
|
|
existing.replace_backend(sandbox_backend)
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
if repo:
|
|
|
|
|
existing.bound_repo = repo
|
2026-05-11 16:03:38 -07:00
|
|
|
return existing
|
|
|
|
|
|
|
|
|
|
proxy = SandboxBackendProxy(sandbox_backend)
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
if repo:
|
|
|
|
|
proxy.bound_repo = repo
|
2026-05-11 16:03:38 -07:00
|
|
|
SANDBOX_BACKENDS[thread_id] = proxy
|
|
|
|
|
return proxy
|
|
|
|
|
|
|
|
|
|
|
fix: resolve security-review findings (sandbox isolation, IAM list scope, webhook replay, info-leak) (#54)
* fix: enforce a replay window on Linear webhooks (AUTHZ-001)
verify_linear_signature accepted any correctly-signed body with no freshness
check, so a captured request could be replayed indefinitely. Parse the
signed webhookTimestamp (Unix ms) and reject requests outside a 60s window,
failing closed when the field is missing or malformed — mirroring the Slack
verifier.
* fix: stop leaking upstream auth-error bodies into user comments
get_github_token_for_user folded the raw upstream response text into the
error string that becomes a Slack/Linear comment (AUTH-RESP-LEAK-01). Log the
full body server-side only and return a generic "GitHub auth failed (status
<code>)". Also document the accepted shared-installation-token blast radius on
the bot-token-only path (AUTHZ-003).
* fix: bind sandbox and token caches to repo to prevent thread-id collision
A PR head-branch name is attacker-controllable and get_thread_id_from_branch
derives a thread_id from its first UUID with no repo binding (TID-COLLIDE-01).
The in-memory sandbox cache and the per-thread GitHub-token cache were keyed on
thread_id alone, and a cached sandbox was reused after only an echo-ping, so a
different repo's webhook could bind to another thread's sandbox or token.
Without changing the persistent thread-id scheme:
- Persist the bound repo (owner/name) in thread metadata on sandbox creation and
refuse to reuse a sandbox whose bound repo does not match the current event
(SandboxRepoMismatchError); the in-memory proxy also carries the binding.
- Bind the GitHub-token cache entries to their repo and evict on a cross-repo
read so a colliding thread_id cannot be served another repo's token.
- Thread repo through the reviewer and the webhook token resolvers.
* fix: scope s3:ListBucket to the releases/ prefix (F-1/IAC-04)
The instance role and the GitHub deploy app role granted s3:ListBucket on the
whole assets bucket. Every caller (deploy.sh, the publish/rollback scripts)
only ever lists under releases/, so add a StringLike s3:prefix=releases/*
condition. GetBucketLocation has no s3:prefix in its request context, so it
moves to its own unconditioned statement. Also document the accepted F-2
cross-env existence-oracle residual on BatchGetSecretValue.
* chore: suppress test-fixture credential false positive; document AUTHZ-002
Add a machine-level suppression for the fake Datadog key in the
test_team_credentials encryption-roundtrip fixture (CWE-798, not a real
credential). Clarify that the within-org thread-write path is intentional by
design (AUTHZ-002) — comment only, no behavior change.
* fix: casefold repo-binding keys to avoid spurious cross-repo mismatch
GitHub owner/name are case-insensitive. Casefold the owner/name key on both the
write (binding) and read (compare) sides — repo_cache_key and the metadata
bound_repo read — so Org/Repo and org/repo resolve to one repo and a legitimate
same-repo run cannot raise a spurious SandboxRepoMismatchError (Gap 2).
* fix: stop leaking upstream auth body in unexpected-result branch
The 2xx-but-missing-token/url branch echoed the parsed upstream response body
into the user-facing error. Return a generic message and log response_data
server-side only, mirroring the existing HTTPStatusError fix (Gap 4).
* fix: fail closed for unbound-legacy sandboxes and catch repo mismatch
Gap 1: a thread with a persisted sandbox_id but no in-memory cache and no
recorded bound_repo (a pre-binding legacy thread, post-deploy) previously
reconnected-and-served the sandbox to the current repo, then rebound it. Now
fail closed: drop the stale id and recreate a fresh sandbox bound to this repo,
logging a reconnect-with-missing-binding event. A sandbox is never served to a
repo unless its binding is known and matches; new threads bind on first run
unchanged.
Gap 3: catch SandboxRepoMismatchError at the agent and reviewer run entrypoints,
log it for alarming, and surface a clean sanitized error instead of letting an
opaque deep-stack exception crash-loop the worker.
* chore: suppress test-fixture credential false positive in token-TTL tests
Add a machine-level suppression for the fake "ghp_secret" GitHub token used by
the cached-token TTL/revocation unit tests (CWE-798). Not a real credential and
not a valid PAT; scoped to the unit test only.
2026-06-29 12:21:19 -04:00
|
|
|
async def get_bound_repo_from_metadata(thread_id: str) -> str | None:
|
|
|
|
|
"""Fetch the repo (``owner/name``) this thread's sandbox is bound to."""
|
|
|
|
|
try:
|
|
|
|
|
config = get_config()
|
|
|
|
|
except Exception:
|
|
|
|
|
return None
|
|
|
|
|
metadata = config.get("metadata", {})
|
|
|
|
|
if not isinstance(metadata, dict):
|
|
|
|
|
return None
|
|
|
|
|
bound_repo = metadata.get("bound_repo")
|
|
|
|
|
# Casefold on read so legacy metadata written before normalization (e.g.
|
|
|
|
|
# ``Org/Repo``) still compares equal to the casefolded current repo key.
|
|
|
|
|
return repo_cache_key(bound_repo) if isinstance(bound_repo, str) and bound_repo else None
|
|
|
|
|
|
|
|
|
|
|
2026-05-11 16:03:38 -07:00
|
|
|
def clear_sandbox_backend(thread_id: str) -> None:
|
|
|
|
|
SANDBOX_BACKENDS.pop(thread_id, None)
|
2026-02-17 15:03:20 -08:00
|
|
|
|
|
|
|
|
|
2026-02-17 16:50:08 -08:00
|
|
|
async def get_sandbox_id_from_metadata(thread_id: str) -> str | None:
|
2026-02-17 15:03:20 -08:00
|
|
|
"""Fetch sandbox_id from thread metadata."""
|
|
|
|
|
try:
|
2026-02-17 16:50:08 -08:00
|
|
|
config = get_config()
|
2026-02-17 15:03:20 -08:00
|
|
|
except Exception:
|
2026-02-17 16:50:08 -08:00
|
|
|
logger.exception("Failed to read thread metadata for sandbox")
|
2026-02-17 15:03:20 -08:00
|
|
|
return None
|
2026-05-11 16:03:38 -07:00
|
|
|
metadata = config.get("metadata", {})
|
|
|
|
|
if not isinstance(metadata, dict):
|
|
|
|
|
return None
|
|
|
|
|
sandbox_id = metadata.get("sandbox_id")
|
|
|
|
|
return sandbox_id if isinstance(sandbox_id, str) else None
|
2026-02-17 15:03:20 -08:00
|
|
|
|
|
|
|
|
|
2026-05-11 16:03:38 -07:00
|
|
|
async def get_sandbox_backend(thread_id: str) -> SandboxBackendProxy:
|
2026-02-17 15:03:20 -08:00
|
|
|
"""Get sandbox backend from cache, or connect using thread metadata."""
|
|
|
|
|
sandbox_backend = SANDBOX_BACKENDS.get(thread_id)
|
|
|
|
|
if sandbox_backend:
|
|
|
|
|
return sandbox_backend
|
|
|
|
|
|
2026-02-17 16:50:08 -08:00
|
|
|
sandbox_id = await get_sandbox_id_from_metadata(thread_id)
|
2026-02-17 15:03:20 -08:00
|
|
|
if not sandbox_id:
|
2026-02-17 16:50:08 -08:00
|
|
|
raise ValueError(f"Missing sandbox_id in thread metadata for {thread_id}")
|
2026-02-17 15:03:20 -08:00
|
|
|
|
2026-03-17 11:55:36 -07:00
|
|
|
sandbox_backend = await asyncio.to_thread(create_sandbox, sandbox_id)
|
2026-05-11 16:03:38 -07:00
|
|
|
return set_sandbox_backend(thread_id, sandbox_backend)
|
2026-02-17 15:03:20 -08:00
|
|
|
|
|
|
|
|
|
2026-05-11 16:03:38 -07:00
|
|
|
def get_sandbox_backend_sync(thread_id: str) -> SandboxBackendProxy:
|
2026-02-17 15:03:20 -08:00
|
|
|
"""Sync wrapper for get_sandbox_backend."""
|
|
|
|
|
return asyncio.run(get_sandbox_backend(thread_id))
|