Add team pages#1161
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Team section to the marketing app: header/footer links to /team, a team listing page, individual team member pages with JSON‑LD and metadata, sitemap entries for /team and each member, a TeamMemberCard component and barrel export, content front‑matter updates, a redirect from /about → /team, and a new font in layout. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant NextJS as Next.js Server
participant PeopleLib as getPersonById / getAllPeople
participant Renderer as TeamMember React Server Component
Browser->>NextJS: GET /team or /team/{id}
NextJS->>PeopleLib: fetch people data (getAllPeople / getPersonById)
PeopleLib-->>NextJS: people / person data (or null)
alt person found
NextJS->>Renderer: render page with person + allPeople
Renderer-->>NextJS: HTML with JSON-LD, metadata, and links
NextJS-->>Browser: 200 HTML
else not found
NextJS-->>Browser: 404
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/marketing/content/people/satya.mdx`:
- Around line 1-8: The avatar field in this markdown frontmatter uses satya.webp
while other team member files (e.g., avi.mdx, kiet.mdx) use .jpg, so standardize
the avatar format: choose the target extension (e.g., .webp or .jpg) and update
the avatar value in this file's frontmatter (avatar: /team/satya.webp → avatar:
/team/satya.jpg or vice versa) and ensure the actual image asset in the team
directory is renamed/converted to match; verify other team member files use the
same extension so all avatar references are consistent.
In `@apps/marketing/src/app/team/`[id]/page.tsx:
- Around line 60-64: The JSON-LD injection uses dangerouslySetInnerHTML with
JSON.stringify(schema) which doesn't escape characters like <, >, &; change the
rendering to produce an escaped JSON string before passing to
dangerouslySetInnerHTML (escape <, >, & in the JSON output) or replace
JSON.stringify(schema) with a safe serializer such as serialize-javascript;
update the script tag that references schema (the dangerouslySetInnerHTML usage
in page.tsx) to use the escaped/serialized string so closing tags or
script-breakout vectors cannot be injected.
🧹 Nitpick comments (2)
apps/marketing/src/lib/people.ts (1)
48-51: Consider logging when the people directory is missing.For consistency with the error logging in
getPersonById, consider adding a log statement when the directory doesn't exist. This aids debugging when content is expected but the directory is misconfigured.♻️ Optional: Add debug logging for missing directory
export function getAllPeople(): Person[] { if (!fs.existsSync(peopleDirectory)) { + console.warn("[people/getAllPeople] Directory not found:", peopleDirectory); return []; }apps/marketing/src/app/team/page.tsx (1)
29-41: Extract the vertical guide background into named constants (and reuse across team pages).
The inline gradient embeds repeated magic numbers and is duplicated in the team member page, making it harder to tune. Consider hoisting the offsets/color into constants and referencing a single background string.As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.♻️ Suggested refactor
+const GUIDE_OFFSET_PX = 384; +const GUIDE_LINE_PX = 1; +const GUIDE_COLOR = "rgba(255,255,255,0.06)"; +const VERTICAL_GUIDE_BACKGROUND = `linear-gradient( + to right, + transparent 0%, + transparent calc(50% - ${GUIDE_OFFSET_PX}px), + ${GUIDE_COLOR} calc(50% - ${GUIDE_OFFSET_PX}px), + ${GUIDE_COLOR} calc(50% - ${GUIDE_OFFSET_PX - GUIDE_LINE_PX}px), + transparent calc(50% - ${GUIDE_OFFSET_PX - GUIDE_LINE_PX}px), + transparent calc(50% + ${GUIDE_OFFSET_PX - GUIDE_LINE_PX}px), + ${GUIDE_COLOR} calc(50% + ${GUIDE_OFFSET_PX - GUIDE_LINE_PX}px), + ${GUIDE_COLOR} calc(50% + ${GUIDE_OFFSET_PX}px), + transparent calc(50% + ${GUIDE_OFFSET_PX}px) +)`; + export default function TeamPage() { const people = getAllPeople(); return ( <main className="relative min-h-screen"> {/* Vertical guide lines */} <div className="absolute inset-0 pointer-events-none" style={{ - backgroundImage: ` - linear-gradient(to right, transparent 0%, transparent calc(50% - 384px), rgba(255,255,255,0.06) calc(50% - 384px), rgba(255,255,255,0.06) calc(50% - 383px), transparent calc(50% - 383px), transparent calc(50% + 383px), rgba(255,255,255,0.06) calc(50% + 383px), rgba(255,255,255,0.06) calc(50% + 384px), transparent calc(50% + 384px)) - `, + backgroundImage: VERTICAL_GUIDE_BACKGROUND, }} />
| --- | ||
| name: Satya Patel | ||
| role: Cofounder | ||
| avatar: /team/satya.webp | ||
| twitter: saddle_paddle | ||
| github: saddlepaddle | ||
| linkedin: saddlepaddle | ||
| --- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the directory structure
echo "=== Checking for team directory ==="
find . -type d -name team | head -20
echo -e "\n=== Looking for avatar files ==="
fd -t f . | grep -i "team.*\.(jpg|jpeg|webp|png)$" | head -20
echo -e "\n=== Checking apps/marketing/public/team directory ==="
ls -la apps/marketing/public/team/ 2>/dev/null || echo "Directory not found"
echo -e "\n=== Checking satya.mdx ==="
find . -name "satya.mdx" -exec cat {} \;
echo -e "\n=== Checking avi.mdx ==="
find . -name "avi.mdx" -exec cat {} \;
echo -e "\n=== Checking kiet.mdx ==="
find . -name "kiet.mdx" -exec cat {} \;Repository: superset-sh/superset
Length of output: 1057
Consider using a consistent avatar file format across team members.
Team avatars use inconsistent formats: satya.webp while avi.mdx and kiet.mdx use .jpg. Consider standardizing on a single format (e.g., all .jpg or all .webp) for consistency and easier maintenance of the team directory.
🤖 Prompt for AI Agents
In `@apps/marketing/content/people/satya.mdx` around lines 1 - 8, The avatar field
in this markdown frontmatter uses satya.webp while other team member files
(e.g., avi.mdx, kiet.mdx) use .jpg, so standardize the avatar format: choose the
target extension (e.g., .webp or .jpg) and update the avatar value in this
file's frontmatter (avatar: /team/satya.webp → avatar: /team/satya.jpg or vice
versa) and ensure the actual image asset in the team directory is
renamed/converted to match; verify other team member files use the same
extension so all avatar references are consistent.
| <script | ||
| type="application/ld+json" | ||
| // biome-ignore lint/security/noDangerouslySetInnerHtml: Safe for JSON-LD structured data | ||
| dangerouslySetInnerHTML={{ __html: JSON.stringify(schema) }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JSON.stringify escape less-than character < security risk JSON-LD
💡 Result:
JSON.stringify() is not an HTML-escaping function. It escapes JSON-required characters (quotes, backslashes, control chars), but it does not reliably protect you when you embed the resulting JSON directly into an HTML <script> element (including JSON-LD: <script type="application/ld+json">…</script>).
Why < / </script> matters (XSS risk)
Inside an HTML <script> element, the HTML parser can terminate the element when it encounters </script>—even if that text appears “inside” a JavaScript/JSON string. OWASP explicitly calls out that a </script> closing tag will close the script block because the HTML parser runs before the JS parser. [1] This is exactly how JSON-in-script XSS happens (e.g., attacker-controlled text contains </script><script>…). [2]
So the risk is not “JSON-LD is unsafe”; it’s: embedding untrusted data into a <script> tag without HTML-safe escaping is unsafe.
What to do
When emitting JSON/JSON-LD into a <script> tag, post-process the serialized JSON to escape characters that can trigger HTML/script parsing issues—at minimum < (commonly also >, &, '). This widely used pattern prevents </script> from ever appearing as raw text by turning < into \u003c. [2] Many template engines do exactly this (e.g., Jinja’s tojson). [2]
Safe-ish JS pattern
const jsonLd = JSON.stringify(data)
.replace(/</g, "\\u003c")
.replace(/>/g, "\\u003e")
.replace(/&/g, "\\u0026")
.replace(/'/g, "\\u0027");(Then place jsonLd between the <script type="application/ld+json"> tags.)
Alternative
Use a library designed to make JSON safe for embedding in <script> tags; OWASP JSON Sanitizer, for example, guarantees the output won’t contain the substring </script (case-insensitive). [3]
When it’s a real problem
- Problem: any JSON-LD fields include untrusted content (user profiles, reviews, comments, product Q&A, etc.).
- Usually fine: all values are fully server-controlled and never include attacker input.
Sources: OWASP XSS Prevention Cheat Sheet (script-tag termination) [1]; examples and recommended escaping (< → \u003c) [2]; OWASP JSON Sanitizer guarantee about </script [3].
[1] (docs.devnetexperttraining.com)
[2] (stackoverflow.com)
[3] (owasp.org)
Citations:
- 1: https://docs.devnetexperttraining.com/static-docs/OWASP-Cheat-Sheet-Series/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html?utm_source=openai
- 2: https://stackoverflow.com/questions/39193510/how-to-insert-arbitrary-json-in-htmls-script-tag?utm_source=openai
- 3: https://owasp.org/json-sanitizer/?utm_source=openai
🌐 Web query:
React dangerouslySetInnerHTML JSON-LD script type application/ld+json XSS prevention
💡 Result:
To emit JSON-LD in React you typically do use a <script type="application/ld+json"> with dangerouslySetInnerHTML, but you must ensure the string cannot break out of the script tag and become executable HTML/JS.
Key safety rules
- Never inject untrusted/user-controlled strings into
dangerouslySetInnerHTMLunless you first sanitize/validate them (React’s normal escaping is bypassed). [1][2] - Generate JSON-LD as an object, then
JSON.stringifyit (don’t build JSON with string concatenation). [2] - Escape
<in the JSON output so an attacker can’t inject</script>via a value and terminate the tag (a common breakout). This is a widely used pattern (including in Next.js docs). [2] - Add defense-in-depth: CSP to reduce XSS impact if something slips through. [4]
- Google explicitly supports JSON-LD in a
<script>tag and can read it even when injected via JS (so this approach is compatible with SEO requirements). [3]
Recommended React snippet (safe-by-construction pattern)
const jsonLd = {
"@context": "https://schema.org",
"@type": "Product",
name: product.name,
description: product.description,
// keep values trusted/validated
};
<script
type="application/ld+json"
dangerouslySetInnerHTML={{
__html: JSON.stringify(jsonLd).replace(/</g, "\\u003c"),
}}
/>This uses JSON.stringify plus < escaping to prevent </script>-style breakouts. [2]
If any fields might be user-controlled
- Treat them as untrusted input and apply strict validation/allow-listing (preferred), or sanitization where appropriate, because
dangerouslySetInnerHTMLreintroduces XSS risk. [1][5]
Sources: React’s warning about dangerouslySetInnerHTML and XSS risk [1], Next.js JSON-LD implementation pattern with < escaping [2], Google structured data/JSON-LD guidance [3], OWASP CSP guidance [4], OWASP XSS prevention guidance [5].
🌐 Web query:
JSON-LD script injection prevention best practices React
💡 Result:
Best practices to prevent JSON-LD <script> injection (React)
-
Treat JSON-LD data as untrusted input
- Don’t place user-controlled strings directly into JSON-LD without validation/normalization (same principle as any XSS defense). Avoid using
dangerouslySetInnerHTMLfor anything except this narrowly-scoped JSON-LD use case. (React’s protection is bypassed when you inject raw HTML/script.) [5]
- Don’t place user-controlled strings directly into JSON-LD without validation/normalization (same principle as any XSS defense). Avoid using
-
Escape characters that can break out of the
<script>tag- A classic payload is
</script>inside a JSON string, which can terminate the tag and allow HTML/script injection. - At minimum, escape sequences that could form a closing tag, e.g. replace
</with<\/before embedding. [4] - Next.js explicitly warns that
JSON.stringifyalone doesn’t prevent XSS in JSON-LD and suggests scrubbing<(e.g. to\u003c) or using a safer serializer. [1]
- A classic payload is
-
Prefer a serializer designed for embedding in
<script>- Use
serialize-javascript(in JSON mode if applicable). It automatically escapes HTML characters and line terminators so</script>becomes unicode-escaped. [6] - Pin to a patched version and monitor advisories:
serialize-javascripthad an XSS issue affecting>= 6.0.0 < 6.0.2, patched in6.0.2. [7] - Do not enable the library’s “unsafe” mode/options that disable escaping. [6]
- Use
-
Add a strict Content Security Policy (CSP) as a backstop
- Use nonce- or hash-based
script-src(avoid relying only on domain allowlists). [3] - If you must use an inline JSON-LD
<script>, allow it via a nonce (or hash), not'unsafe-inline'. [2][3]
- Use nonce- or hash-based
Minimal React example (safe embedding pattern)
import serialize from "serialize-javascript";
const JsonLd = ({ data, nonce }) => (
<script
type="application/ld+json"
nonce={nonce}
dangerouslySetInnerHTML={{ __html: serialize(data, { isJSON: true }) }}
/>
);(Uses escaping designed for <script> embedding [6] and works well with nonce-based CSP [2][3].)
Harden JSON‑LD injection against potential script breakout.
JSON.stringify doesn't escape <, so if any person fields ever become user-controlled, </script> could break out of the tag. Currently low-risk since this data is repo-controlled, but add escaping as a defensive measure for future-proofing.
🛡️ Suggested hardening
- const schema = {
+ const schema = {
"@context": "https://schema.org",
"@type": "Person",
name: person.name,
jobTitle: person.role,
url,
worksFor: {
"@type": "Organization",
name: COMPANY.NAME,
url: COMPANY.MARKETING_URL,
},
...(person.avatar && {
image: `${COMPANY.MARKETING_URL}${person.avatar}`,
}),
...(sameAs.length > 0 && { sameAs }),
};
+ const jsonLd = JSON.stringify(schema)
+ .replace(/</g, "\\u003c")
+ .replace(/>/g, "\\u003e")
+ .replace(/&/g, "\\u0026");
return (
<script
type="application/ld+json"
// biome-ignore lint/security/noDangerouslySetInnerHtml: Safe for JSON-LD structured data
- dangerouslySetInnerHTML={{ __html: JSON.stringify(schema) }}
+ dangerouslySetInnerHTML={{ __html: jsonLd }}
/>
);Escape <, >, & to block HTML/script breakout vectors. If untrusted content enters person fields, consider using a dedicated library like serialize-javascript instead.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 62-62: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
In `@apps/marketing/src/app/team/`[id]/page.tsx around lines 60 - 64, The JSON-LD
injection uses dangerouslySetInnerHTML with JSON.stringify(schema) which doesn't
escape characters like <, >, &; change the rendering to produce an escaped JSON
string before passing to dangerouslySetInnerHTML (escape <, >, & in the JSON
output) or replace JSON.stringify(schema) with a safe serializer such as
serialize-javascript; update the script tag that references schema (the
dangerouslySetInnerHTML usage in page.tsx) to use the escaped/serialized string
so closing tags or script-breakout vectors cannot be injected.
Resolve conflict in sitemap.ts by keeping both team pages and comparison pages.
- Add bios for Satya, Kiet, and Avi with links to their previous companies - Support HTML rendering in bios for clickable company links - Add permanent redirect from /about to /team
2 participants
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit