8 commits

Author	SHA1	Message	Date
Adam Moussa	c99bd78179	feat: clean-review auto-approve for unsolicited verdicts [BLOCKED — security] (#217) ... * feat(reviewer): clean-review auto-approve for unsolicited publish_review verdicts An unsolicited publish_review(verdict="approve") — a run dispatched without verdict_requested — is now honored when the review has zero open findings, so clean auto-reviews land a real APPROVE. With open findings it downgrades to a comment review (verdict_ignored_reason= "approve_with_open_findings"). request_changes stays explicit-request- only; the self-review, head-moved, and author-unknown downgrades and the shell verdict guard are unchanged. The reviewer base prompt now instructs the clean-approve call on auto-reviews. * chore(security): record accepted-risk suppressions for clean-review auto-approve Two confirmed-HIGH findings from /sh-security-review on the clean-review auto-approve change are accepted and deferred (Adam, 2026-07-21), tracked in #218. Machine-recorded per the mandatory-security-review policy; the revisit trigger is promotion from dev to main/prod.	2026-07-21 20:18:25 -04:00
Adam Moussa	7f60324f0c	chore: decommission self-hosted AWS LangGraph stack (#64) ... * chore: decommission self-hosted AWS LangGraph stack Removes the now-dead self-host IaC and AWS-only CI/CD after destroying the dev + prod CloudFormation stacks (open-swe-dev, open-swe-prod, open-swe-iam, and the dev-exclusive CDKToolkit-oswedev bootstrap) in account 328440206208, us-east-1. The deployment is now managed (LangGraph Cloud + Vercel). - remove infra/ (CDK app: app + IAM stacks, constructs, aspects, tests) - remove deploy/ami (Packer AMI build) and deploy/seahaven (boot/config scripts, DEPLOYMENT/ROTATION runbooks) - remove AWS-only workflows: cd-infra, ci-infra, build-artifacts, rollback - README: rewrite the Deployment section to the managed LangGraph Cloud + Vercel view; drop dead links to infra/ and deploy/seahaven Preserved: the shared default CDKToolkit bootstrap and promote-dev-to-prod.yml. The RETAIN'd Secrets Manager shells and open-swe-<env>-assets S3 buckets survive cdk destroy by design (orphaned) and need a separate deliberate cleanup. * chore: clean up dangling references left by the AWS decommission Folds in the FIX-level items from the #64 review gates (GPT-4.1 cross-review + /sh-security-review), none of which were blockers: - delete orphaned .github/scripts/{package-artifacts,publish-and-deploy,roll-box, rollback}.sh — their only callers were the removed AWS deploy workflows - drop the deleted /infra dir from dependabot.yml npm directories (was producing a recurring Dependabot config error) - remove the stale OSWE-IAC-SECRETS-LIST-01 suppression (referenced the deleted infra/lib/constructs/instance-role.ts) - repoint the README promotion link to promote-to-main.yml (renamed in #63) The promote-dev-to-prod.yml comment in check-dev-green.sh is intentionally left to #63, which rewrites that same line.	2026-06-29 19:54:38 -04:00
Adam Moussa	8a9974c3c4	feat: author Slack/dashboard/schedule commits + PRs as the app by default (#57) (#60) ... * feat: default Slack/dashboard/schedule PRs + commits to the app identity (#57) Slack/dashboard/schedule runs now author PRs and run git/gh operations as the GitHub App seahaven-openswe[bot] by default (matching GitHub-issue runs), so the self-review 422 is impossible by construction rather than guarded in the prompt. A profile flag author_prs_as_user restores per-user attribution. - open_pull_request._resolve_pr_author_token + auth.resolve_github_token: default to the installation token for these sources; per-user only when opted in. - authorship: commit identity -> seahaven-openswe[bot] (numeric noreply; accepted Vercel-resolution risk, documented inline). - self-trigger safety: INTERNAL_BOT_LOGINS + webapp/reviewer_reconcile/reply markers recognize seahaven-openswe[bot] (bot-authored events are now ours). Supersedes the prompt-only guard in #58. * fix: author commits as the app bot in the default path (SH-IDSPLIT-01) Security review found the commit identity was NOT actually unified to the bot: resolve_triggering_user_identity got a 403 from the installation token and fell back to configurable['github_login'], so commits were still authored as the triggering user (commit=user, push+PR=bot — a three-way split that missed the stated goal). Now gate the triggering-user identity resolution on the same default-bot decision as the token: slack/dashboard/schedule default to the app bot identity unless author_prs_as_user is set. * docs(security): record AUTHZ-SLACK-BOT-DEFAULT-001 as an accepted residual (#59) Single-user deployment; bounded by App-on-pilot + ALLOWED_GITHUB_REPOS lock. Revisit (add a per-user gate) before expanding users or the App installation.	2026-06-29 14:22:33 -04:00
Adam Moussa	134963647b	chore(security): suppress pre-existing history scanner false-positives (#56) ... Adds repo-local suppressions for 5 verified-FP gitleaks findings that block pushes (forcing --no-verify), all in committed history / docs / CI fixtures: - .env.ci (dev-only e2e values, intentionally committed) - .env.example (placeholders) - .github/ci/fake_github_app_key.pem (throwaway CI test key) - INSTALLATION.md (example GITHUB_APP_PRIVATE_KEY .env block) - README.md (prose mis-matched by the generic-api-key heuristic) Repo-local (not machine-level) so they load in git worktrees too. Also drops the now-obsolete OSWE-IAC-AUDIT-01 suppression (B-1, fixed in #55).	2026-06-29 12:54:40 -04:00
Adam Moussa	a33aaec495	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
Adam Moussa	cdcb6625b9	fix: BatchGetSecretValue must be on * for the filtered batch call (#23) ... The prior fix scoped secretsmanager:BatchGetSecretValue to the env-prefixed secret ARN, but the live box still got AccessDenied: batch-get-secret-value invoked WITH a name --filters is a COLLECTION call that AWS authorizes against * (a per-secret ARN does not satisfy it). Split the statement: - GetSecretValue + DescribeSecret stay PREFIX-scoped (secret:open-swe-<env>/*) — this is what gates which secret VALUES the box can read (checked per-secret in the batch). - BatchGetSecretValue + ListSecrets move to a * operation-level statement (the filtered collection call + the list action; neither is resource-scopable for this usage). VALUE isolation preserved (dev box still cannot read prod secret values); only secret NAME/metadata enumeration is widened. GPT-4.1 IAM cross-review: BLOCK none, FIX none. Suppression OSWE-IAC-SECRETS-LIST-01 updated; future hardening (explicit --secret-id-list to drop both * grants) tracked there.	2026-06-26 19:44:12 -04:00
Adam Moussa	cfbdcda9b7	fix: instance role BatchGetSecretValue + ListSecrets for .env materialization (#22) ... * fix(infra): grant instance role BatchGetSecretValue + ListSecrets for .env materialization fetch-config.sh materializes the box's .env via `secretsmanager batch-get-secret-value --filters Key=name,Values=open-swe-<env>/`, but the instance role only granted GetSecretValue/DescribeSecret. BatchGetSecretValue is a distinct IAM action, so the call was AccessDenied and open-swe.service crash-looped (no .env written -> ExecStartPre exit 1). - Add secretsmanager:BatchGetSecretValue to the prefix-scoped ReadSecrets statement. - Add secretsmanager:ListSecrets on * (required by the name-prefix filtered batch call; the API has no resource-level scoping for the list action — fits the role's stated exception). Secret VALUES stay prefix-scoped; only names are enumerable. Reviews: GPT-4.1 IAM cross-review BLOCK=none; /sh-security-review iac-iam one LOW metadata residual (no critical/high), recorded as OSWE-IAC-SECRETS-LIST-01. Refs T7/T19 dev bring-up. * ci: lift Node heap cap for Playwright E2E build (vite OOM) The E2E job's Playwright globalSetup runs the real `bun run build`, whose vite bundle exceeds Node's default ~2 GB heap and OOMs (JavaScript heap out of memory) — the same failure fixed for build-artifacts.yml in #19. Set NODE_OPTIONS=--max-old-space-size=8192 on the Run E2E step.	2026-06-26 19:29:50 -04:00
Adam Moussa	404b3f6f75	feat: stand up dev properly — assets bucket + artifact CD + baked AMI + on-box uv sync (T7+T19+T14) (#18) ... * feat(infra): build + pin the baked open-swe-base-arm64 AMI (T12 AMI / item 3) Packer-build the custom base image and repoint AppService off the AL2023 placeholder onto it. deploy/ami/open-swe-base.pkr.hcl — fix two bugs that blocked the first real `packer build` (the config had only ever been `packer validate`'d at T8): - the file provisioner failed uploading the templates dir ('scp: …: Is a directory') — a trailing-slash contents-upload needs the dest dir to exist; added a 'mkdir -p /tmp/open-swe-templates' shell provisioner + dropped the dest trailing slash. - the shell provisioner's custom execute_command omitted {{ .Vars }}, so the environment_vars never reached provision.sh (which runs under set -u and aborted on CLOUDWATCH_AGENT_DEB_URL). Added {{ .Vars }}. infra: - ami-cache.ts: BAKED_OPEN_SWE_AMI_ID = ami-0545363bb147229ff (built 2026-06-26 from open-swe-base-arm64-20260626-201929) + bakedOpenSweArm64() pinning it by exact id via MachineImage.genericLinux (offline, deterministic). Dropped the now-dead AL2023 cachedInContext helper + context key; kept the EBS/replacement discipline docs. - app-service.ts: machineImage → bakedOpenSweArm64(). - open-swe-stack.ts: output BakedAmiId (was the AL2023 PinnedAmiId guard). - cdk.context.json → {} (AMI is a static id pin; no context lookups remain). - README: Baked AMI + EBS-replacement-discipline section. tsc + cdk synth(dev+prod) + jest(16) clean; template ImageId = the baked AMI. NOTE: held — do NOT merge until the open-swe-dev secret values are populated (put-config.sh). The infra CD is live, so merging this to dev auto-deploys OpenSweDevStack; without secrets the box boots but fetch-config fail-fasts → unhealthy ALB target on the shared prod ALB. Merge once secrets are set (T14). * fix(ami): ASCII-only AMI description + re-pin to ami-00080084502093021 Third packer bug: ami_description had an em-dash (non-ASCII); AWS rejects non-ASCII in the AMI Description attribute, so packer registered then DEREGISTERED the first AMI (ami-0545…) on the ModifyImageAttribute error. Replaced with an ASCII '-'. Rebuilt clean → ami-00080084502093021 (available). Re-pinned BAKED_OPEN_SWE_AMI_ID. * fix(deploy): GitHub App + Slack required for prod only, not dev Per the migration decision: do NOT create/duplicate a separate dev GitHub App or Slack app — only prod owns the single shared app. So fetch-config.sh no longer hard-requires the GitHub App quintet (ID/PRIVATE_KEY/INSTALLATION_ID/CLIENT_ID/ CLIENT_SECRET) + Slack/webhook secrets for dev; they move into the prod-only block alongside the existing GITHUB_WEBHOOK_SECRET/SLACK_SIGNING_SECRET. Dev now boots with just DASHBOARD_JWT_SECRET + TOKEN_ENCRYPTION_KEY + the active provider key(s) + the langsmith sandbox keys. Dev is a deployment-validation env (boot/health/boundary) with no GitHub/Slack/webhook integration; prod parity is unchanged (prod still requires everything). * feat: stand up dev properly — S3 assets bucket + artifact CD + on-box uv sync (T7+T19) Make the dev/prod box deployable end-to-end: a real artifact pipeline and a re-runnable on-box deploy, so OpenSweDevStack can come up genuinely healthy. Infra (T7): - assets-bucket.ts: open-swe-<env>-assets S3 bucket — BLOCK_ALL public access, SSE-S3, enforceSSL (deny non-TLS), versioned, lifecycle (expire noncurrent + abort MPU), RETAIN. Wired into OpenSweStack + CfnOutput. - app-service.ts: open-swe-<env>-deploy SSM document that runs the baked /opt/open-swe/bin/deploy.sh (tag-scoped roll-the-box). machineImage is the baked open-swe-base-arm64 AMI (folds in the held #16). IAM (app deploy role — cross-review gated): - github-deploy-roles.ts: app role gains s3:PutObject/DeleteObject scoped to open-swe-<env>-assets/releases/* (CI uploads releases). Drops the generic AWS-RunShellScript grant now that the dedicated open-swe-<env>-deploy document is the only SendCommand path — closes the T4 BLOCK#3 arbitrary-shell timebox. Boot/deploy (T19): - deploy/ami/deploy.sh: single, re-runnable app-deploy procedure — pull app.tar.gz/spa.tar.gz from S3, `uv sync --frozen --no-dev` (native ARM64 venv at the real path, py3.12 pre-baked), restart open-swe.service + reload nginx. - user-data.sh: nginx starts BEFORE the app deploy (static /healthz -> the ALB target is healthy even before the first release); deploy.sh is base64-rendered by CDK into user-data (a normal reviewable repo file, not a heredoc) and the first-boot deploy is NON-FATAL (no release yet -> wait for the first SSM deploy). CI (T7+T19): - build-artifacts.yml (+ .github/scripts): build the SPA with bun (vite -> ui/.output/public -> spa.tar.gz), package the Python source via git archive (app.tar.gz, no ui/ no .venv), upload to releases/<sha>/ + releases/latest/ via the githubdeploy-open-swe-app-<env> OIDC role, then fire open-swe-<env>-deploy. push dev -> dev (auto); push main -> prod (env "prod" approval gate). Local: ruff/shellcheck clean, tsc clean, jest 16/16, cdk synth offline OK, deploy.sh base64 round-trips exact. * harden(sec-review): tar extraction, deploy gating, least-privilege, secret guard Address the /sh-security-review fan-out + proof-or-kill verifier pass. Only one confirmed-high surfaced and it is PRE-EXISTING and out-of-diff (OSWE-IAC-AUDIT-01, the account-wide CDK cfn-exec residual already documented in config.ts; recorded in .security-review/suppressions.json with justification + flagged for the per-env bootstrap-qualifier follow-up). The rest were verifier-downgraded to unverified; these are the cheap defense-in-depth fixes worth taking regardless: - deploy.sh: extract tarballs with --no-same-owner --no-same-permissions (root never honors an archive's uid/mode → no setuid/foreign-owned file can land); and treat "no release in S3 yet" as a benign exit 0, distinct from a real deploy failure (set -e stays loud once a release exists). - publish-and-deploy.sh: gate on the AGGREGATE SSM Command.Status (+ TargetCount), not CommandInvocations[0], so a partial failure across the brief 2-instance replacement window can't be reported as success. - instance-role.ts: scope the box's s3:GetObject to releases/* (mirrors the app role's write scope) instead of the whole bucket. - package-artifacts.sh: fail-closed secret-shaped-file guard on app.tar.gz (defense in depth over .gitignore; scoped to data extensions so *_credentials.py source is not a false positive — verified against the real tree). Deferred as documented follow-ups (verifier: unverified, supply-chain-gated to the CI OIDC writer; bucket is BLOCK_ALL + enforceSSL + versioned): SHA-pinned immutable releases/<sha>/ pulls + signed checksum (vs mutable latest/), single-tarball release to remove the torn-read window, and app-aware ALB health (vs static nginx /healthz). shellcheck/tsc/jest(16) clean; both stacks synth offline. * fix(infra): ASCII-only EC2 SecurityGroup descriptions + synth-time guard The instance-SG GroupDescription + ingress/egress rule descriptions carried an em-dash / arrow (—, →). `tsc` and `cdk synth` accept them, but the EC2 API rejects non-ASCII in GroupDescription ("Character sets beyond ASCII are not supported"), so OpenSweDevStack's first deploy failed at the SG and rolled back. (Pre-existing from #14; same class as the AMI-description ASCII bug.) - app-service.ts: replace —/→ with ASCII (- / ->) in the SG GroupDescription, the ingress/egress rule descriptions, and the Route53 comment. - test/ascii-aws-fields.test.ts: synth-time guard asserting EC2 SecurityGroup GroupDescription + rule descriptions are pure ASCII, so this fails the build instead of a deploy next time. jest 18/18; tsc clean. * fix(infra): SG rule descriptions use ASCII-charset-safe text (no `>`) The first ASCII fix replaced the arrow with `->`, but EC2 SecurityGroup *rule* descriptions allow a stricter set than ASCII — `a-zA-Z0-9. _-:/()#,@[]+=&;{}!$*`, which EXCLUDES `<`/`>`. So OpenSweDevStack's second deploy still failed at the ingress rule. Use "to" instead of "->", and tighten the guard test from "ASCII only" to the exact EC2 allowed charset so it catches `>` (and `<`) too. jest 18/18; tsc clean. * fix(infra): minify embedded deploy.sh so user-data fits EC2's 25.6 KB limit The base64 deploy.sh embedded in user-data pushed the encoded boot script to 27184 bytes, over EC2's 25600-byte cap, so OpenSweDevStack's instance failed with "Encoded User data is limited to 25600 bytes". Strip full-line comments + blank lines from deploy.sh before base64-embedding it (repo file keeps comments; only the on-box copy is minified; the script is opaque base64 so user-data heredocs are unaffected) -> rendered user-data drops to 16424 bytes (9 KB margin). Add a synth-time guard test asserting EC2 user-data stays under 25600 bytes encoded. jest 19/19; minified deploy.sh passes bash -n + shellcheck.	2026-06-26 18:49:09 -04:00