Skip to content

feat(cdp): let the attachment upload route through without the internal api secret - #133482

Merged
trunk-io[bot] merged 2 commits into
masterfrom
feat/node-store-upload-auth-scope
Nov 11, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
feat/node-store-upload-auth-scope

Conversation

@Silthus

@Silthus Silthus commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Merge gate: passed. fleet proof gate passes on ef4c5f1 (live run, review at HEAD, CI green, judge pass). Layer 1 of 4 in the node-store attachment stack: this PR, then #131833, #133495, and #131944. The stack merges from its top PR.

Problem

  • Developers who attach files to a transactional email will upload them through a new cdp-api route first (PostHog/michaelr-context#77).
  • The shared-secret middleware refuses every request without INTERNAL_API_SECRET, so that route could never be reached.

Changes

  • The middleware lets exactly /api/projects/{team_id}/transactional_email/attachments/uploads through, as it already does for the other team-scoped transactional email routes.
  • Nothing serves that path until feat(cdp): add the upload route for transactional email files #131833 lands, so behavior today is unchanged.
  • A sibling path such as .../uploads/extra still needs the shared secret.

How did you test this code?

Test rationale: the new case drives a real express app through setupExpressApp. It fails if the pattern is dropped, and also if it widens to sibling paths. Deleting the pattern turned the test red.

  • Live run over real HTTP against an app from setupExpressApp: the exempted path answers 404 without the secret, since nothing serves it yet. Its /extra sibling, a trailing slash, and another team route still answer 401.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /writing-tests, /writing-pr-descriptions, /stacking-prs

More context for AI agents

🤖 Generated with Claude Code

…al api secret

The upload route checks its own team-scoped token, as the transactional email routes do, so the shared-secret middleware lets exactly that path through. Nothing serves the path until the route itself lands.

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trunk-io

trunk-io Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

😎 This pull request was merged.

@Silthus Silthus self-assigned this Nov 11, 2026
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

@Silthus
Silthus added this pull request to stack #133496 November 11, 2026 11:12
@Silthus
Silthus marked this pull request as ready for review November 11, 2026 11:40
@stamphog
stamphog Bot requested a review from a team November 11, 2026 11:40
@parameterai

parameterai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Risk: Low · 1 low

This PR exempts exactly one new path (/api/projects/{team_id}/transactional_email/attachments/uploads) from the nodejs shared-secret middleware, ahead of the handler that PR #131833 will add. The pattern is precisely anchored, no current route serves the path (it 404s today), and every parser-differential shape (trailing slash, /extra siblings, case, percent-encoding) fails closed back to the shared secret. The one real risk is that the exemption merges decoupled from the per-handler JWT check that justifies it, violating the middleware file's own stated invariant.

Sentinel reviewed ef4c5f1 · Review settings

@veria-ai

veria-ai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

PR overview

No security issues were found in this pull request.

Reviews

@Silthus
Silthus requested a review from dmarchuk November 11, 2026 13:39
…oad-auth-scope

Refs PostHog/michaelr-context#77

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trunk-io

trunk-io Bot commented Nov 11, 2026

Copy link
Copy Markdown

This pull request is queued for merge as part of 131944, which will merge 133482, 131833, 133495, 131944.

@trunk-io
trunk-io Bot merged commit fb2aa66 into master Nov 11, 2026
208 checks passed
@trunk-io
trunk-io Bot deleted the feat/node-store-upload-auth-scope branch November 11, 2026 15:57
@trunk-io

trunk-io Bot commented Nov 11, 2026

Copy link
Copy Markdown

This pull request was merged into master as part of stacked PR 131944.

@deployment-status-posthog

deployment-status-posthog Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-11 16:27 UTC Run
prod-us ✅ Deployed 2026-11-11 18:14 UTC Run
prod-eu ✅ Deployed 2026-11-11 17:23 UTC Run

This table shows the posthog release only. Hogstation shows every service that this PR reaches, Kargo services included.

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.

2 participants