Skip to content

fix: glasshouse security sweep (GH-043, GH-287, GH-083, GH-084) - #3579

Merged
tofikwest merged 12 commits into
mainfrom
dennis/security-sweep
Oct 2, 2026
Merged

tofikwest merged 12 commits into
mainfrom
dennis/security-sweep

Conversation

@dennisofficial

@dennisofficial dennisofficial commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

This completes the repo-level fixes for GH-43, GH-287, GH-83, and GH-84 while preserving the newer security fixes on main. Production uses Comp v2; this repository remains publicly maintained.

Changes

  • Improvements, new tables #43: use a shared strict organization-ID allowlist for vector filters. Remove browser task-trigger tokens: the legacy token endpoint returns 410, and the old answer-question, vendor-questionnaire-orchestrator, and parse-questionnaire task IDs reject all requests. Legitimate parsing uses the guarded questionnaire API and a new internal task; browser run observation remains read-only.
  • refactor: improve ControlsTable component structure and readability #287: suggestions require the requested active organization, a current active membership, and task:read, vendor:read, and evidence:read permissions before reading vendor/context data. Built-in and custom roles use the central permission resolver.
  • Lewis/policies #83: disable the Next.js image optimizer in app and portal. Remote avatars, logos, and signed uploads load directly in the browser; server-side image resizing/caching is intentionally disabled.
  • feat(policies): Enhance policies dashboard with comprehensive overvie… #84: pin third-party Actions and Bun, enable weekly Actions updates, retire the Gram publishing and secret-bearing Claude review workflows, and add deterministic security regression CI. Preserve and harden main's broader regression workflow and release-only SDK publishing under the legacy npm tag.
  • Merge current main and the teammate's newer PR commits, resolving overlaps without dropping their security protections or test coverage.

Validation

  • 226 API tests passed across 18 suites, including questionnaire tenant isolation, retired tasks, vector injection, active membership, permission guards, and earlier security regressions.
  • 111 app tests passed across 9 suites, including suggestions permissions, image endpoint behavior, questionnaire parsing, task status, and remediation batches.
  • 43 GitHub workflow-policy tests passed.
  • ESLint passed for the new API security modules/tests and vector integration files; whitespace checks passed.
  • GitHub confirms this head is conflict-free. Hosted regression CI did not run because repository Actions are disabled. Main still requires the retired security-review status, so normal merge remains blocked by that setting.
  • Repository-wide lint/typecheck still fail on existing formatting, workspace-module resolution, and unrelated test/type errors. These checks are not claimed green.

Operational follow-up

Updated Trigger workers must be deployed for the rejecting legacy task registrations to replace existing live task implementations. Repository administrators should remove unused workflow credentials and any obsolete required Security Review check; see .github/SECURITY_CI.md. No production/Comp v2 changes, deployments, credential rotation, or repository settings changes were performed. This PR addresses the listed findings; it does not establish that all repository dependency alerts are resolved.

findSimilarContent/findSimilarContentBatch interpolated a caller-controlled
organizationId into the Upstash Vector filter DSL, letting any authenticated
user inject quote/OR/GLOB payloads (or another tenant's id) and read
embedded policy/knowledge-base chunks cross-tenant via the answer-question
trigger task. Upstash filters have no bound parameters, so validate against
the strict org_<cuid> pattern before interpolation and fail closed. The same
interpolation in sync-organization's embedding verification gets the helper.

Refs GH-43
…uggestions

The server action queried vendors and context Q&A with a caller-controlled
organizationId and no authentication; the edge proxy only checks for the
presence of a session cookie, so a dummy cookie plus any org id exposed an
org's vendor inventory and knowledge-base Q&A through the LLM output. The
action now resolves the better-auth session server-side and returns nothing
unless the requested org is the session's active organization, matching the
sibling task-automation actions.

Refs GH-287
Both apps configured next/image remotePatterns with hostname '**', and both
proxies exclude _next/image from auth, turning the optimizer into an
unauthenticated server-side fetcher for arbitrary https URLs (reachability
oracle, content laundering, bandwidth abuse). Remote sources still rendered
through the optimizer are img.logo.dev integration logos and S3-hosted org
assets; every dynamic org/integration asset already renders with the
unoptimized flag and bypasses remotePatterns, so the wildcard is not needed.

Refs GH-83
19 of 20 third-party uses: refs resolved at run time from floating branches
and tags, including anthropics/claude-code-security-review@main (holds
ANTHROPIC_API_KEY and pull-requests:write on every PR) and
repo-sync/pull-request@v2 (contents/pull-requests/issues write). All refs
are now pinned to the commit sha of the version they floated on (checkout
v4.3.0, setup-bun v2.2.0, repo-sync v2.12.1, discord v1.20.0, speakeasy
v15.60.11; the claude review action has no tags so it is pinned to the
current main sha), and gram-sync moves GRAM_API_KEY from job env into the
push step so the curl|bash installer step never receives it.

Refs GH-84
@CLAassistant

CLAassistant commented Sep 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dennisofficial
dennisofficial marked this pull request as ready for review September 24, 2026 21:24

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 27 files

Confidence score: 2/5

  • generate-suggestions.ts checks organization identity but not API RBAC, so members without task:read may invoke the action and access vendor/context data; enforce the API permission check before returning data.
  • apps/app/next.config.ts restricts remote images while some dynamic <Image> call sites remain optimized, which can break image rendering; add unoptimized to the affected integration, organization-logo, and avatar images.
  • apps/portal/next.config.ts still permits the image optimizer to proxy arbitrary AWS service hostnames, including attacker-controlled endpoints; restrict the allowlist to the application’s actual S3 endpoints.
  • The release workflows and .github/actions/bun-install/action.yml still use bun-version: latest, making toolchains non-reproducible, while SHA-pinned actions lack an automated update path; pin Bun versions and configure action updates.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/generate-suggestions.ts">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/tasks/[taskId]/automation/[automationId]/actions/generate-suggestions.ts:42">
P1: This guard checks organization identity but never enforces API RBAC. A member whose custom role lacks `task:read` can directly invoke this Server Action and read vendor/context data; call the API permission check before these queries.</violation>
</file>

<file name="apps/app/next.config.ts">

<violation number="1" location="apps/app/next.config.ts:61">
P2: This allowlist breaks existing dynamic remote images because several `<Image>` call sites still omit `unoptimized`, contrary to the comment. Add `unoptimized` to dynamic integration, organization-logo, and avatar images, or allow their exact trusted hosts before narrowing this list.</violation>
</file>

<file name=".github/workflows/database-migrations-release.yml">

<violation number="1" location=".github/workflows/database-migrations-release.yml:20">
P2: The action is pinned to a commit SHA, but `bun-version: latest` still installs a floating Bun runtime, so this workflow (which runs `bunx prisma migrate deploy` against `DATABASE_URL_PROD`) stays non-reproducible and could pick up a breaking or compromised Bun release. Sibling workflows already pin `bun-version: "1.3.4"` (e.g., trigger-tasks-deploy-release.yml); do the same here to fully satisfy the GH-084 supply-chain goal.</violation>
</file>

<file name=".github/actions/bun-install/action.yml">

<violation number="1" location=".github/actions/bun-install/action.yml:26">
P3: `setup-bun` is now pinned to a SHA, but the input immediately below still uses `bun-version: latest`, so the installed Bun runtime remains a floating version. In a hardening/reproducibility sweep this leaves the toolchain unpinned: a new Bun release can silently change install/lockfile behavior. Pin it to the exact version the repo currently works with (e.g., `bun-version: <current-version>`), kept in sync with the action's pinned tag.</violation>
</file>

<file name="apps/portal/next.config.ts">

<violation number="1" location="apps/portal/next.config.ts:26">
P2: This wildcard still leaves the image optimizer as an unauthenticated proxy for any AWS service hostname, including attacker-controlled S3 website or API Gateway endpoints. Restrict it to the application’s actual S3 endpoint forms or mark dynamic URLs `unoptimized`, as the comment describes.</violation>
</file>

<file name=".github/workflows/trigger-tasks-deploy-release.yml">

<violation number="1" location=".github/workflows/trigger-tasks-deploy-release.yml:15">
P2: These new full-SHA pins have no automated update path: `.github/dependabot.yml` configures only the `npm` ecosystem, so nothing will refresh the pinned actions (including future security fixes in `actions/checkout`, `actions/setup-node`, and `oven-sh/setup-bun`). GitHub's hardening guidance pairs SHA pinning with Dependabot version updates for `github-actions`, so add that ecosystem to `.github/dependabot.yml`; the `# v4.3.0`-style comments are exactly what Dependabot uses to bump SHA pins.</violation>
</file>

<file name=".github/workflows/device-agent-release.yml">

<violation number="1" location=".github/workflows/device-agent-release.yml:107">
P2: The setup-bun action is now pinned to a commit SHA, but it still installs `bun-version: latest`, so the bun toolchain the release binaries are built with floats between runs. That leaves builds non-reproducible and keeps a floating third-party dependency in the exact supply-chain surface GH-084 is hardening. Pin bun to an explicit version (e.g. `bun-version: 1.2.x`) matching the lockfile, and update it deliberately alongside the action SHA.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread apps/app/next.config.ts Outdated
Comment thread .github/workflows/database-migrations-release.yml
Comment thread apps/portal/next.config.ts Outdated
Comment thread .github/workflows/trigger-tasks-deploy-release.yml
Comment thread .github/workflows/device-agent-release.yml
Comment thread .github/actions/bun-install/action.yml
@dennisofficial
dennisofficial requested a review from Marfuen October 2, 2026 01:14
@tofikwest
tofikwest merged commit 5005eb2 into main Oct 2, 2026
3 checks passed
@tofikwest
tofikwest deleted the dennis/security-sweep branch October 2, 2026 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants