Repository navigation
#346 — render: link PR number + title in the change-context banner - #363
Conversation
--pr-url/--pr-title/--pr-number flags + [pr] TOML section (flags override config); review-mode derives them from gh pr view (same call, +title/url). Renders PR #<n> linked in the --pr-diff banner (a-href = zero-egress-safe navigation, title askama-escaped). Synthetic config-pr-showcase fixture + regenerated diff-showcase golden; jaffle/playground/macro-heavy byte-identical. Closes #346 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 31 minutes and 18 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements optional PR-context banners for ChangesPR context banner linking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Ready to review this PR? Stage has broken it down into 7 individual chapters for you: 
Chapters generated by Stage for commit 87ddb76 on Jun 13, 2026 8:14pm UTC. |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch 🧭 Explore previewThe two-page 🟡 Golden exploreThe committed
🐶 Live exploreThis PR doesn't touch ▶ Open ↗ opens the report or explorer in your browser in one The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27477844847 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a feature (cute-dbt#346) that links the change-context banner in the generated report to the source pull request. It introduces new CLI flags (--pr-url, --pr-title, --pr-number) and a [pr] configuration section, while also automatically extracting PR details from gh pr view during review runs. The PR title is properly HTML-escaped in the template to prevent XSS. However, a security review identified a potential XSS vulnerability where an unvalidated PR URL could be used to inject malicious pseudo-protocols like javascript: or data: into the href attribute. It is recommended to validate that the resolved URL starts with a safe scheme like http:// or https://.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The `[`PrConfig`](crate::domain::PrConfig)` link is redundant — PrConfig is imported at module scope, so `[`PrConfig`]` auto-resolves to the same item. Trips rustdoc::redundant-explicit-links under -D warnings (the cargo-doc CI gate). Doc-comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/adapters/render.rs (1)
3062-3092:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict
pr_ref.urlto a safe GitHub PR URL before rendering.Line 3068 and Line 3092 forward the raw URL from CLI/TOML/review into a clickable
hrefand the embedded payload. HTML escaping won't neutralizejavascript:ordata:schemes, so a crafted--pr-urlcan turn the local report into a click-triggered script sink and break the "navigation-only / zero-egress" contract. Canonicalize this from trusted PR parts, or drop anything outsidehttps://github.com/.../pull/....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/render.rs` around lines 3062 - 3092, The pr_ref value assigned into payload.pr_ref (see pr_ref, PrRefPayload::from and the ReportTemplate field pr_ref) must be canonicalized/validated before embedding or rendering: parse the candidate URL and allow only the exact GitHub PR form (scheme https, host github.com, path matching /{owner}/{repo}/pull/{number}); reconstruct a safe canonical URL from owner/repo/number and drop or null out pr_ref if it fails validation; ensure the sanitized/None pr_ref is what is passed into payload.pr_ref and therefore into payload_json_for_html_script and the ReportTemplate so no raw user-provided URL (including javascript: or data: schemes) can be emitted.
🧹 Nitpick comments (2)
src/cli/review.rs (2)
1023-1030: ⚡ Quick winClarify documentation: "manifest" is the wrong term here.
Lines 1024–1025 and 1028 refer to "the manifest" when describing the
titleandurlfields, but these fields are extracted from thegh pr viewJSON response, not from dbt'smanifest.json. Using "manifest" in this context could confuse maintainers who might think this is related to the dbt manifest file.📝 Suggested doc wording
- /// The PR title (cute-dbt#346) — feeds the change-context banner link. - /// Empty when the manifest carries no title (the banner then renders - /// link-free — both a url and a title are required). + /// The PR title (cute-dbt#346) — feeds the change-context banner link. + /// Empty when `gh pr view` returns no title (the banner then renders + /// link-free — both a url and a title are required). pub title: String, - /// The PR's GitHub URL (cute-dbt#346) — the `<a href>` the banner - /// links to. Empty when absent (banner renders link-free). + /// The PR's GitHub URL (cute-dbt#346) — the `<a href>` the banner + /// links to. Empty when `gh pr view` returns no URL (banner renders link-free). pub url: String,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/review.rs` around lines 1023 - 1030, Update the field docs for the struct fields title and url in src/cli/review.rs (the pub title: String and pub url: String lines) to replace the word "manifest" with a clearer source description such as "the GitHub PR view JSON (gh pr view response)" or "the GitHub PR JSON returned by `gh pr view`", so the comments read that title and url are extracted from the GitHub PR view JSON response rather than from dbt's manifest.json.
1699-1699: 💤 Low valueOptional: Avoid cloning
pr_infoby reordering.Line 1699 clones
facts.pr_infobefore moving it into the returned tuple. Sincefactsis only used once more (line 1700), you could reorder to avoid the clone:♻️ Suggested reordering
let facts = gather_base_facts(toplevel, args.base.as_deref())?; - let pr_info = facts.pr_info.clone(); let (base, rung) = decide_base(&facts)?; + let pr_info = facts.pr_info; (base, rung, pr_info)The performance gain is small (one avoided allocation per review run), but the code is equally clear.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/review.rs` at line 1699, Avoid the unnecessary clone of facts.pr_info: instead move it out of facts (e.g., let pr_info = facts.pr_info; or destructure with let Facts { pr_info, .. } = facts;) and then use the remaining fields of facts as before; this eliminates the extra allocation while preserving behavior—ensure that subsequent use of facts only accesses other fields (or borrow as needed) so moving pr_info is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/steps/pr_diff_scoping.rs`:
- Around line 1178-1194: The test banner_shows_escaped_title currently only
asserts that raw_title (if it contains '<') does not appear verbatim in html;
update it to also assert that an escaped form or a safe substring of the title
is present so the title is actually rendered and escaped. Concretely, compute a
safe_part from raw_title (e.g., keep alphanumeric and whitespace characters as
suggested) and if safe_part.trim() is non-empty assert that
html.contains(&safe_part), and additionally verify the escaped form for '<' by
checking html.contains(&raw_title.replace("<", "<")) (and similarly for other
common escapes like '>' -> ">", '&' -> "&", '"' -> """) so the test
both fails if the title was omitted and ensures proper escaping; make these
checks inside banner_shows_escaped_title using the existing raw_title and html
variables.
---
Outside diff comments:
In `@src/adapters/render.rs`:
- Around line 3062-3092: The pr_ref value assigned into payload.pr_ref (see
pr_ref, PrRefPayload::from and the ReportTemplate field pr_ref) must be
canonicalized/validated before embedding or rendering: parse the candidate URL
and allow only the exact GitHub PR form (scheme https, host github.com, path
matching /{owner}/{repo}/pull/{number}); reconstruct a safe canonical URL from
owner/repo/number and drop or null out pr_ref if it fails validation; ensure the
sanitized/None pr_ref is what is passed into payload.pr_ref and therefore into
payload_json_for_html_script and the ReportTemplate so no raw user-provided URL
(including javascript: or data: schemes) can be emitted.
---
Nitpick comments:
In `@src/cli/review.rs`:
- Around line 1023-1030: Update the field docs for the struct fields title and
url in src/cli/review.rs (the pub title: String and pub url: String lines) to
replace the word "manifest" with a clearer source description such as "the
GitHub PR view JSON (gh pr view response)" or "the GitHub PR JSON returned by
`gh pr view`", so the comments read that title and url are extracted from the
GitHub PR view JSON response rather than from dbt's manifest.json.
- Line 1699: Avoid the unnecessary clone of facts.pr_info: instead move it out
of facts (e.g., let pr_info = facts.pr_info; or destructure with let Facts {
pr_info, .. } = facts;) and then use the remaining fields of facts as before;
this eliminates the extra allocation while preserving behavior—ensure that
subsequent use of facts only accesses other fields (or borrow as needed) so
moving pr_info is valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71d068dd-e9c8-420e-8700-8239ef652288
📒 Files selected for processing (16)
.github/workflows/ci.yml.github/workflows/report-preview.ymlexamples/README.mdexamples/diff-showcase-report.htmlfeatures/pr_diff_scoping.featuresrc/adapters/render.rssrc/cli/args.rssrc/cli/mod.rssrc/cli/review.rssrc/domain/config.rssrc/domain/mod.rstemplates/report.htmltests/fixtures/MANIFEST.tomltests/fixtures/config-pr-showcase.tomltests/headless_toggle.rstests/steps/pr_diff_scoping.rs
… review)
gemini security review: PrConfig.url interpolates into the change-context
banner's <a href>. askama escapes the title's HTML metacharacters but NOT a
url's scheme, so a javascript:/data: url from an untrusted [pr] config would
execute on click — an XSS in the otherwise trivially-auditable-safe report.
resolve() now allows only http(s) (case-insensitive); any other scheme (or a
scheme-relative //host) degrades to a link-free banner. +4 unit tests.
Also strengthens the BDD escaped-title step (CodeRabbit) with a positive
assertion that the title is actually rendered (longest non-escapable segment
present), so a dropped {{ pr.title }} can no longer pass it silently.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧾 Merge debrief — #363 (
|
1 participant
Links the PR in the report change-context banner: PR # — <title> as an to the PR. Gen-time via --pr-url/--pr-title/--pr-number flags + a [pr] TOML section (flags override config); the review subcommand derives them from gh pr view (same call + title/url). Zero-egress-safe ( = navigation, fires nothing at view-time; title askama-escaped). Synthetic config-pr-showcase fixture + regenerated diff-showcase golden; jaffle/playground/macro-heavy byte-identical.
NOTE: committed by the orchestrator after the builder finished + verified clippy/fmt/nextest(2005)/bdd(222)/headless+zero-egress locally but was disk-blocked before crap4rs + push. CI runs the full suite.
Closes #346
Summary by CodeRabbit
Release Notes
New Features
--pr-url,--pr-title,--pr-numberCLI arguments or[pr]TOML config section.cute-dbt reviewcommand to automatically capture and pass PR metadata from GitHub CLI.Documentation