Skip to content

fix: Metabase embed on /careers showing "Message seems corrupt or manipulated" #2829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
felixkrrr wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −7
Open

fix: Metabase embed on /careers showing "Message seems corrupt or manipulated" #2829

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions app/api/metabase-embed/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,35 @@
import { NextRequest, NextResponse } from "next/server";
import jwt from "jsonwebtoken";

const METABASE_SITE_URL = "https://langfuse.metabaseapp.com";

export async function GET(request: NextRequest) {
try {
const METABASE_SITE_URL = "https://langfuse.metabaseapp.com";
const METABASE_SECRET_KEY = process.env.METABASE_SECRET_KEY;
const secretKeyRaw = process.env.METABASE_SECRET_KEY;

if (!METABASE_SECRET_KEY) {
console.error("Missing required environment variables: METABASE_SECRET_KEY");
if (!secretKeyRaw) {
console.error("Missing required environment variable: METABASE_SECRET_KEY");
return NextResponse.json(
{ message: "Server configuration error" },
{ status: 500 }
);
}

const secretKey = secretKeyRaw.trim();
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 21, 2026

Choose a reason for hiding this comment

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

P2 Empty string not guarded after trim

If METABASE_SECRET_KEY is set to a whitespace-only string (e.g. " \n"), the !secretKeyRaw guard passes because a non-empty string is truthy. After .trim(), the value becomes "". jsonwebtoken accepts an empty string and produces a token, but Metabase will reject it with the same "corrupt or manipulated" error. A post-trim emptiness check would close this gap and surface a clearer error message in the logs.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: app/api/metabase-embed/route.ts
Line: 18

Comment:
**Empty string not guarded after trim**

If `METABASE_SECRET_KEY` is set to a whitespace-only string (e.g. `"  \n"`), the `!secretKeyRaw` guard passes because a non-empty string is truthy. After `.trim()`, the value becomes `""`. `jsonwebtoken` accepts an empty string and produces a token, but Metabase will reject it with the same "corrupt or manipulated" error. A post-trim emptiness check would close this gap and surface a clearer error message in the logs.

How can I resolve this? If you propose a fix, please make it concise.


const dashboardId = request.nextUrl.searchParams.get("dashboardId")
? parseInt(request.nextUrl.searchParams.get("dashboardId")!, 10)

Check warning on line 21 in app/api/metabase-embed/route.ts

View check run for this annotation

Claude / Claude Code Review

Whitespace-only secret key bypasses config-error guard 

The emptiness check runs on `secretKeyRaw` (before trimming), so a whitespace-only `METABASE_SECRET_KEY` passes the guard and then trims to `""`, causing `jwt.sign()` to throw and return a generic 500 "Internal Server Error" instead of the intended "Server configuration error". Move the check to after the trim: replace `if (\!secretKeyRaw)` with `const secretKey = secretKeyRaw.trim();` first, then `if (\!secretKey)`.
Comment on lines +10 to 21
Copy link
Copy Markdown

@claude claude Bot Apr 21, 2026

Choose a reason for hiding this comment

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

🟡 The emptiness check runs on secretKeyRaw (before trimming), so a whitespace-only METABASE_SECRET_KEY passes the guard and then trims to "", causing jwt.sign() to throw and return a generic 500 "Internal Server Error" instead of the intended "Server configuration error". Move the check to after the trim: replace if (\!secretKeyRaw) with const secretKey = secretKeyRaw.trim(); first, then if (\!secretKey).

Extended reasoning...

Bug: The guard if (\!secretKeyRaw) in app/api/metabase-embed/route.ts (line 10) runs on the raw, untrimmed environment variable value. A non-empty but whitespace-only value such as ' ' or '\n' is truthy in JavaScript, so the early-return guard is skipped entirely.

Code path: After the guard passes, const secretKey = secretKeyRaw.trim() runs (line 21), producing secretKey = "". The empty string is then passed directly to jwt.sign(payload, secretKey). In jsonwebtoken v9.x, the library internally checks if (\!secretOrPrivateKey) and throws Error: secretOrPrivateKey must have a value.

Why existing code doesn't prevent it: The outer try/catch block does catch the thrown error, but it falls into the generic handler which returns { message: "Internal Server Error" } with status 500 — not the intentional { message: "Server configuration error" } path with its console.error("Missing required environment variable: METABASE_SECRET_KEY") log line that would aid diagnosis.

Impact: If someone accidentally pastes only whitespace into the Vercel env var UI for METABASE_SECRET_KEY, the embed endpoint returns a confusing generic 500 with no targeted log message, making it harder to diagnose the root cause. The PR's primary fix (trimming a real key that has surrounding whitespace) works correctly; this is a logical ordering nit for the degenerate all-whitespace case.

Fix: Swap the order — call .trim() first, then check the trimmed result:

const secretKey = (process.env.METABASE_SECRET_KEY ?? '').trim();
if (\!secretKey) {
  console.error('Missing required environment variable: METABASE_SECRET_KEY');
  return NextResponse.json({ message: 'Server configuration error' }, { status: 500 });
}

Step-by-step proof:

  1. Operator sets METABASE_SECRET_KEY=" " (a single space) in Vercel.
  2. secretKeyRaw = " " — a truthy string of length 1.
  3. \!secretKeyRaw evaluates to false; the guard does not return early.
  4. secretKey = secretKeyRaw.trim() → secretKey = "".
  5. jwt.sign(payload, "") → jsonwebtoken throws Error: secretOrPrivateKey must have a value.
  6. Caught by outer catch; response is 500 Internal Server Error with no targeted log.
  7. Expected behaviour (with fix): \!secretKey is true → clean 500 Server configuration error with the descriptive console.error log.

: 25;
const theme = request.nextUrl.searchParams.get("theme") === "day" ? "day" : "night";
const theme =

Check notice on line 23 in app/api/metabase-embed/route.ts

View check run for this annotation

Claude / Claude Code Review

parseInt dashboardId produces NaN payload on non-numeric input 

Pre-existing issue: passing a non-numeric `dashboardId` query parameter (e.g., `?dashboardId=abc`) causes `parseInt` to return `NaN`, which `JSON.stringify` serializes as `null`, resulting in a JWT payload of `{ resource: { dashboard: null } }` that Metabase will reject. In practice this is only reachable via a crafted direct API request since the only internal caller always passes a static integer; adding an `isNaN` guard with a fallback to `25` would harden the endpoint.
Comment on lines 20 to +23
Copy link
Copy Markdown

@claude claude Bot Apr 21, 2026

Choose a reason for hiding this comment

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

🟣 Pre-existing issue: passing a non-numeric dashboardId query parameter (e.g., ?dashboardId=abc) causes parseInt to return NaN, which JSON.stringify serializes as null, resulting in a JWT payload of { resource: { dashboard: null } } that Metabase will reject. In practice this is only reachable via a crafted direct API request since the only internal caller always passes a static integer; adding an isNaN guard with a fallback to 25 would harden the endpoint.

Extended reasoning...

What the bug is

At app/api/metabase-embed/route.ts lines 20-23, the dashboardId query parameter is parsed with parseInt(..., 10). The ternary guard only checks whether the parameter is present/truthy — it does not validate that the value is actually numeric. If a caller passes a non-numeric string, parseInt returns NaN.

The problematic serialization path

NaN is not a valid JSON value. When JSON.stringify serializes an object containing NaN, it silently converts it to null:

JSON.stringify({ x: NaN }) // '{x:null}'

jsonwebtoken serializes the payload as JSON internally, so a request with ?dashboardId=abc produces the JWT payload { resource: { dashboard: null }, params: {}, exp: ... }. Metabase's embed endpoint expects dashboard to be a valid integer and rejects the token.

Why existing code does not prevent it

The ternary request.nextUrl.searchParams.get('dashboardId') ? parseInt(...) : 25 only guards against a missing or empty parameter. A non-numeric value like 'abc' is truthy, so the branch is taken and parseInt('abc', 10) returns NaN without any further check.

Impact

For the normal application flow the impact is zero: the single internal caller (components/MetabaseDashboard.tsx) is TypeScript-typed with dashboardId: number and hardcodes the value 25. However, the API route is publicly reachable, so any crafted request with a non-numeric dashboardId will receive a 200 response containing a JWT that Metabase immediately rejects — yielding a confusing error rather than a clean 400.

Concrete step-by-step proof

  1. GET /api/metabase-embed?dashboardId=abc
  2. searchParams.get('dashboardId') returns 'abc' (truthy) → ternary takes the parseInt branch
  3. parseInt('abc', 10) → NaN
  4. dashboardId = NaN; payload becomes { resource: { dashboard: NaN }, ... }
  5. jwt.sign(payload, secretKey) serializes payload as JSON → { resource: { dashboard: null }, ... }
  6. Metabase receives a token with dashboard: null and returns an error

Relationship to this PR

This logic was not changed by the PR (which only trims the secret key and updates cache headers). It is a pre-existing issue, but the PR touches the same file and function, making it a natural opportunity to add the guard.

Fix

const raw = request.nextUrl.searchParams.get('dashboardId');
const parsed = raw ? parseInt(raw, 10) : NaN;
const dashboardId = \!isNaN(parsed) ? parsed : 25;

Or return a 400 if an invalid value is explicitly supplied.

request.nextUrl.searchParams.get("theme") === "day" ? "day" : "night";

const payload = {
resource: { dashboard: dashboardId },
params: {},
exp: Math.round(Date.now() / 1000) + 10 * 60,
};

const token = jwt.sign(payload, METABASE_SECRET_KEY);
const token = jwt.sign(payload, secretKey);
const iframeUrl = `${METABASE_SITE_URL}/embed/dashboard/${token}#theme=${theme}&bordered=false&titled=false`;

return NextResponse.json(
Expand All @@ -35,7 +39,7 @@
},
{
headers: {
"Cache-Control": "public, s-maxage=300, max-age=0",
"Cache-Control": "no-store, no-cache, must-revalidate",
},
}
);
Expand Down
Loading