feat(cli-v2): add fern docs diff command#16067
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
| }); | ||
|
|
||
| if (token) { | ||
| await page.setExtraHTTPHeaders({ |
There was a problem hiding this comment.
Puppeteer's page.setExtraHTTPHeaders() attaches the specified headers to every network request the page context makes, including cross-origin sub-resource requests (CDNs, analytics, fonts, third-party scripts). Setting FERN_TOKEN here means it will be forwarded to every domain that the preview page or production docs page fetches resources from. An attacker who controls any third-party resource loaded by the page (or can observe traffic to a CDN it loads) receives the bearer token and gains full API access as the authenticated user.
Prompt To Fix With AI
Replace `page.setExtraHTTPHeaders` with Puppeteer request interception so the FERN_TOKEN header is added only to requests targeting the same origin as the page URL.
```ts
await page.setRequestInterception(true);
const targetHost = new URL(url).host;
page.on('request', (req) => {
const reqHost = new URL(req.url()).host;
const extraHeaders = reqHost === targetHost && token
? { ...req.headers(), FERN_TOKEN: token }
: req.headers();
req.continue({ headers: extraHeaders });
});
```
Remove the `page.setExtraHTTPHeaders` block entirely. Apply the same change to the v1 implementation at `packages/cli/cli/src/commands/docs-diff/docsDiff.ts`.Severity: high | Confidence: 92%
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const afterPath = join(outputPath, RelativeFilePath.of(`${filename}-after.png`)); | ||
| const comparisonBasePath = join(outputPath, RelativeFilePath.of(`${filename}-comparison.png`)); | ||
|
|
||
| const productionUrl = `${productionUrlInfo.baseUrl}/${slug}`; |
There was a problem hiding this comment.
🔴 Production URL ignores basePath, breaking docs hosted under a subpath
The getProductionUrlInfo function (line 646-673) parses the production URL and extracts a basePath (e.g., /docs for https://example.com/docs/). However, at line 183, the production URL is constructed as ${productionUrlInfo.baseUrl}/${slug} — the basePath is completely ignored. If docs are hosted at https://example.com/docs/, the resulting URL would be https://example.com/{slug} instead of the correct https://example.com/docs/{slug}, causing the "before" screenshot to always hit a 404 for any docs site configured with a basepath.
| const productionUrl = `${productionUrlInfo.baseUrl}/${slug}`; | |
| const productionUrl = productionUrlInfo.basePath != null ? `${productionUrlInfo.baseUrl}${productionUrlInfo.basePath}/${slug}` : `${productionUrlInfo.baseUrl}/${slug}`; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch { | ||
| await page.close(); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🟡 Empty catch block swallows screenshot errors without logging (line 361)
The catch block at line 361 in captureScreenshot catches all errors (including network timeouts, unexpected browser crashes, etc.) and silently returns false without logging the error. This violates the REVIEW.md rule: "No empty catch blocks -- at minimum log the error." When debugging failed diffs, users will have no visibility into why screenshots failed — was it a DNS issue, a timeout, an auth rejection, or a browser crash?
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch { | ||
| return { baseUrl: normalizedUrl, basePath: null }; | ||
| } |
There was a problem hiding this comment.
🟡 Empty catch block swallows URL parsing errors without logging (line 670)
The catch block at line 670 in getProductionUrlInfo silently catches URL parsing errors and returns a fallback value without logging. This violates the REVIEW.md rule: "No empty catch blocks -- at minimum log the error." If the URL from docs.yml is malformed, this silently produces a potentially incorrect baseUrl with no diagnostic output, making misconfiguration hard to debug.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| const browser = await launch({ | ||
| headless: true, | ||
| args: ["--ignore-certificate-errors", "--no-sandbox", "--disable-setuid-sandbox"] |
There was a problem hiding this comment.
The Puppeteer browser is launched with --ignore-certificate-errors, which silently accepts any TLS certificate regardless of validity. The FERN_TOKEN credential is then transmitted in HTTP headers to every page visited (and, per the finding above, to all sub-resources). On any network path where an attacker can intercept traffic — corporate MITM proxy, rogue Wi-Fi, DNS poisoning — the attacker presents a self-signed certificate, the browser accepts it, and the FERN_TOKEN is captured in plain text. Combined with the sub-resource header leak, this creates a reliable credential theft path with no user indication of the interception.
Prompt To Fix With AI
Remove `--ignore-certificate-errors` from the Puppeteer launch args. If self-signed certs are needed for local/preview deployments, handle that case explicitly (e.g., via a `--allow-insecure` CLI flag that the user must opt in to, scoped only to the preview host). Apply the same change to the v1 implementation at `packages/cli/cli/src/commands/docs-diff/docsDiff.ts`.
```ts
const browser = await launch({
headless: true,
args: ["--no-sandbox", "--disable-setuid-sandbox"]
// "--ignore-certificate-errors" removed — it allows MITM to steal FERN_TOKEN
});
```Severity: high | Confidence: 90%
| token: string; | ||
| }): Promise<GetSlugForFileResponse> { | ||
| const filesParam = files.join(","); | ||
| const url = `https://${previewUrl}/api/fern-docs/get-slug-for-file?files=${encodeURIComponent(filesParam)}`; |
There was a problem hiding this comment.
The previewUrl argument is accepted directly from the command line and, after stripping protocol/path, is used as the destination for requests that include FERN_TOKEN in the headers (both the fetch call at line 300–304 and the Puppeteer screenshot at lines 184/339). There is no validation that the hostname belongs to an expected domain (e.g., *.docs.buildwithfern.com). In automated CI pipelines — where the preview URL is often generated from PR metadata or branch names — a malicious actor could inject an attacker-controlled hostname, causing the token to be sent to their server. Capturing the token grants full API access as the CI identity.
Prompt To Fix With AI
Add a domain allowlist check before making any authenticated request to `normalizedPreviewUrl`. For example, validate that the hostname ends with `.docs.buildwithfern.com` or another expected Fern preview domain:
```ts
const ALLOWED_PREVIEW_SUFFIXES = [".docs.buildwithfern.com", ".buildwithfern.com"];
const isAllowed = ALLOWED_PREVIEW_SUFFIXES.some(suffix => normalizedPreviewUrl.endsWith(suffix));
if (!isAllowed) {
throw new CliError({
message: `Preview URL host "${normalizedPreviewUrl}" is not a recognized Fern preview domain.`,
code: CliError.Code.ConfigError
});
}
```
Place this check immediately after `normalizedPreviewUrl` is finalized (after line 137). Apply the same guard in the v1 implementation.Severity: medium | Confidence: 75%
Refactor docs diff command to improve auth, URL normalization and screenshot handling. Replace askToLogin with getToken which supports self-hosted flows via FERN_TOKEN and falls back to context.getTokenOrPrompt. Use URL parsing to normalize preview host and simplify production URL info handling (fall back on invalid URLs). Add runtime response validation for slug API responses. Pass a Logger into captureScreenshot, improve error logging, and adjust screenshot path typing; update side-by-side comparison call. Reorder pngjs/puppeteer deps in package.json and update pnpm lock entries accordingly.
2 participants
Description
Linear ticket: Closes FER-8933
Ports the
fern docs diffcommand to CLI v2. This command generates visual diffs between preview and production docs pages by capturing screenshots and producing side-by-side comparison images.Changes Made
DiffCommandclass atpackages/cli/cli-v2/src/commands/docs/diff/command.tsfollowing the v2 class-based command patterndiffsubcommand under thedocscommand grouppuppeteerandpngjs(+@types/pngjs) dependencies to cli-v2package.jsonCommand usage
The command:
Testing
pnpm turbo run compile --filter=@fern-api/cli-v2passespnpm run check(biome) passesLink to Devin session: https://app.devin.ai/sessions/9b02d3dbec8841ad80d3ded1353ce514
Requested by: @iamnamananand996