mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 12:43:16 +00:00
* 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.
137 lines
6.2 KiB
TypeScript
137 lines
6.2 KiB
TypeScript
import * as iam from "aws-cdk-lib/aws-iam";
|
|
import { Construct } from "constructs";
|
|
import { ACCOUNT, EnvName, REGION, prefix } from "../config";
|
|
|
|
/**
|
|
* Least-privilege EC2 instance role for the open-swe box (one per env).
|
|
*
|
|
* Grants exactly what the boot/runtime flow needs and NOTHING ELSE — no admin,
|
|
* no `*` resources except where the AWS action genuinely has no resource-level
|
|
* scoping. Per-env so the dev box can never read prod secrets/config and vice
|
|
* versa. Reviewed at T4 (GPT-4.1 IAM cross-review) / T5 (/sh-security-review)
|
|
* before it is ever deployed (T6).
|
|
*/
|
|
export class InstanceRole extends Construct {
|
|
public readonly role: iam.Role;
|
|
|
|
constructor(scope: Construct, id: string, env: EnvName) {
|
|
super(scope, id);
|
|
const p = prefix(env);
|
|
|
|
this.role = new iam.Role(this, "Role", {
|
|
roleName: `${p}-instance-role`,
|
|
assumedBy: new iam.ServicePrincipal("ec2.amazonaws.com"),
|
|
description: `EC2 instance role for the ${p} open-swe box (least-privilege).`,
|
|
});
|
|
|
|
// AWS-managed: lets the SSM agent register the instance and RECEIVE the
|
|
// app-deploy `ssm:SendCommand` from githubdeploy-open-swe-app. This is the
|
|
// standard Session-Manager / RunCommand grant and is the only managed
|
|
// policy on the role. DELIBERATE — flag for T4 confirmation.
|
|
this.role.addManagedPolicy(
|
|
iam.ManagedPolicy.fromAwsManagedPolicyName("AmazonSSMManagedInstanceCore"),
|
|
);
|
|
|
|
// Read the build artifact from the env's S3 asset bucket (deploy = pull).
|
|
// Scoped to releases/* — the only prefix CI writes and the box pulls — so a
|
|
// compromised box (or stolen IMDS creds) cannot read anything else that might
|
|
// ever land in the bucket (least-privilege; mirrors the app role's write scope).
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "ReadArtifactObjects",
|
|
actions: ["s3:GetObject"],
|
|
resources: [`arn:aws:s3:::${p}-assets/releases/*`],
|
|
}),
|
|
);
|
|
// ListBucket is constrained to the releases/ prefix (F-1/IAC-04) — the box
|
|
// only ever lists release artifacts, so a compromised box cannot enumerate
|
|
// any other object that might land in the bucket. GetBucketLocation has no
|
|
// s3:prefix in its request context, so it stays a separate, unconditioned
|
|
// statement (the condition would otherwise AccessDeny it).
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "ListArtifactBucket",
|
|
actions: ["s3:ListBucket"],
|
|
resources: [`arn:aws:s3:::${p}-assets`],
|
|
conditions: { StringLike: { "s3:prefix": ["releases/*"] } },
|
|
}),
|
|
);
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "GetArtifactBucketLocation",
|
|
actions: ["s3:GetBucketLocation"],
|
|
resources: [`arn:aws:s3:::${p}-assets`],
|
|
}),
|
|
);
|
|
|
|
// Read non-sensitive config from SSM Parameter Store under /open-swe-<env>/*.
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "ReadSsmConfig",
|
|
actions: ["ssm:GetParameter", "ssm:GetParameters", "ssm:GetParametersByPath"],
|
|
resources: [`arn:aws:ssm:${REGION}:${ACCOUNT}:parameter/${p}/*`],
|
|
}),
|
|
);
|
|
|
|
// VALUE access — Secrets Manager under open-swe-<env>/*. Secret ARNs carry a
|
|
// random 6-char suffix, hence the trailing `*`. This is the statement that
|
|
// actually gates which secret VALUES the box can read: prefix-scoped, so the
|
|
// dev box can never read prod secret values (and vice versa). GetSecretValue is
|
|
// checked per-secret even when the value is returned via the batch call below.
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "ReadSecretValues",
|
|
actions: ["secretsmanager:GetSecretValue", "secretsmanager:DescribeSecret"],
|
|
resources: [`arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:${p}/*`],
|
|
}),
|
|
);
|
|
|
|
// BatchGetSecretValue MUST be granted on `*` — it is a collection action that
|
|
// AWS authorizes against the account, NOT the per-secret ARN, REGARDLESS of
|
|
// whether the caller uses `--filters` or `--secret-id-list`. A prefix-scoped
|
|
// BatchGetSecretValue AccessDenies the whole call ("no identity-based policy
|
|
// allows the secretsmanager:BatchGetSecretValue action") — VERIFIED on the live
|
|
// dev box 2026-06-29 (the OSWE-IAC-SECRETS-LIST-01 attempt to prefix-scope it
|
|
// crash-looped the box once the prior broad grant's eventual-consistency lapsed).
|
|
// This `*` does NOT widen VALUE access: a secret value is only returned when the
|
|
// prefix-scoped GetSecretValue above also allows it, so cross-env value isolation
|
|
// holds. The win that DID survive: fetch-config uses `--secret-id-list` (explicit
|
|
// names, no name filter), so `secretsmanager:ListSecrets` is NOT needed and is
|
|
// intentionally omitted — the box cannot enumerate secret names account-wide.
|
|
// F-2 (accepted residual): because the grant is `*`, a caller naming a secret
|
|
// in ANOTHER env's prefix learns whether that name EXISTS (an existence oracle
|
|
// via the per-secret AccessDenied-vs-not signal) even though the VALUE stays
|
|
// gated by the prefix-scoped GetSecretValue above. Accepted within Sea Haven's
|
|
// single-tenant account 328440206208 — cross-env VALUE isolation is preserved.
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "BatchGetSecretValues",
|
|
actions: ["secretsmanager:BatchGetSecretValue"],
|
|
resources: ["*"],
|
|
}),
|
|
);
|
|
|
|
// NOTE (T11): SSM SecureString + Secrets Manager here are assumed to use the
|
|
// AWS-managed keys (alias/aws/ssm, alias/aws/secretsmanager) for which the
|
|
// service grants Decrypt implicitly — so NO kms:Decrypt is granted. If T11
|
|
// moves these to a customer CMK, add a scoped `kms:Decrypt` on that key ARN
|
|
// ONLY (not `*`).
|
|
|
|
// Ship application logs to CloudWatch Logs under /open-swe/<env>/*.
|
|
this.role.addToPolicy(
|
|
new iam.PolicyStatement({
|
|
sid: "PutAppLogs",
|
|
actions: [
|
|
"logs:CreateLogGroup",
|
|
"logs:CreateLogStream",
|
|
"logs:PutLogEvents",
|
|
"logs:DescribeLogStreams",
|
|
],
|
|
resources: [
|
|
`arn:aws:logs:${REGION}:${ACCOUNT}:log-group:/open-swe/${env}/*`,
|
|
`arn:aws:logs:${REGION}:${ACCOUNT}:log-group:/open-swe/${env}/*:*`,
|
|
],
|
|
}),
|
|
);
|
|
}
|
|
}
|