Enhance embed URL handling and validation system
discourse/discourse@4f8aed29
Golden recall: 100%
Extra findings: 31
Golden Comments (6/6 found)
| # | Comment | Severity | Status | FriendlyReviewer Detail |
|---|---|---|---|---|
| #1 | SSRF vulnerability using open(url) without validation |
HIGH | ✓ Found | "SSRF: invalid_host? validates only the host component, not scheme or port. An attacker can point open(@embed_url) at arbitrary ports (e.g. Redis/SSH) or non-http protocols on the allowlisted host. Validate scheme is http/https and constrain ports" (lib/topic_retriever.rb L15) |
| #2 | The current origin validation using indexOf is insufficient and can be bypassed. An attacker could use a malicious domain like evil-discourseUrl.com to pass this check. |
MEDIUM | ✓ Found | "Origin validation in the postMessage listener uses a fragile substring check (discourseUrl.indexOf(e.origin)) and never checks e.source. Use exact origin comparison and verify e.source === iframe.contentWindow" (app/assets/javascripts/embed.js L13) |
| #3 | postMessage targetOrigin should be the origin (scheme+host+port), not the full referrer URL; using the full URL will cause the message to be dropped and prevent resizing. |
MEDIUM | ✓ Found | "postMessage targetOrigin is the full referer URL instead of just its origin; browsers may normalize but it is incorrect usage and inconsistent with the parent-side exact-origin check. Pass only scheme://host, and guard against blank referer" (app/views/layouts/embed.html.erb L10) |
| #4 | The code sets X-Frame-Options: ALLOWALL which completely disables clickjacking protection. The referer validation can be bypassed (referer headers are easily spoofed), and the fallback to empty string for nil referer masks validation failures. |
MEDIUM | ✓ Found | "Referer-based validation is a weak, spoofable security boundary for a network-fetching endpoint: the only gate for X-Frame-Options: ALLOWALL and job enqueue is request.referer, which is client-controlled. Also produces false negatives when referer is suppressed. Prefer an origin check or HMAC-signed URL" (app/controllers/embed_controller.rb L26) |
| #5 | The TopicEmbed.import method is susceptible to a NoMethodError if the contents parameter is nil when attempting to append a string, and an XSS vulnerability due to unescaped url interpolation in the generated HTML. |
MEDIUM | ✓ Found | Partial match (counted as found): the XSS half is found. "Stored XSS chain: cook returns raw verbatim for raw_html cook_method, bypassing sanitization; the embed view renders it with raw post.cooked (best.html.erb:18); and topic_embed.rb:13 interpolates an unescaped attacker-controlled URL into the 'Imported from' footer HTML" (app/models/post.rb L133). The NoMethodError half is not found: FriendlyReviewer's closest nil-related finding is on the update path (embed.post can be nil, topic_embed.rb L36), not on a nil contents argument to TopicEmbed.import |
| #6 | The ERB block closes with end if, which is invalid Ruby/ERB and will raise at render; it should just be end to close the if block. |
MEDIUM | ✓ Found | "Template contains a Ruby syntax error: <%- end if %> emits end if with no condition, making the best view fail with a 500. Must be <%- end %>" (app/views/embed/best.html.erb L6) |
Supplementary Findings (31 findings)
| Finding | File | Severity | Legitimate? |
|---|---|---|---|
Rate-limit bypass / job flood: best enqueues Jobs::retrieve_topic with no per-IP controller throttle; the Redis throttle is keyed per embed_url, so distinct URLs each get their own key, bypassing the 60s cap and flooding the job queue / triggering repeated outbound fetches |
app/controllers/embed_controller.rb:13 | HIGH | ✓ Real: distinct URLs each get their own throttle key, so the cap is trivially bypassed |
Unhandled nil i.content crashes the whole feed job: i.content.scrub raises NoMethodError for items that only have <description>; no rescue around the item loop, so one malformed item skips all remaining |
app/jobs/scheduled/poll_feed.rb:35 | HIGH | ✓ Real: a malformed feed item kills the entire hourly job |
Migration retry loop silently swallows errors and can spin forever under persistent failure; also rescue PG::Error may not catch ActiveRecord::StatementInvalid on deadlocks in Rails 3.2, defeating the retry |
db/migrate/20131210181901_migrate_word_counts.rb:7 | HIGH | ✓ Real: a silent infinite retry on a migration is a deploy-blocker in disguise |
Missing controller spec for a mismatched referer host: only the absence of a referer is tested; add a case where the referer host differs from embeddable_host and assert 403 / no job enqueue / no ALLOWALL header |
spec/controllers/embed_controller_spec.rb:21 | MEDIUM | ✓ Real: the security-relevant rejection branch is untested |
Retriever spec never exercises no_throttle (staff bypass) or the scheme/port bypass of invalid_host?; retrieved_recently? is always stubbed |
spec/components/topic_retriever_spec.rb:22 | MEDIUM | ✓ Real: the SSRF guard and the throttle bypass are exactly the branches left untested |
iframe src concatenation breaks when discourseUrl lacks a trailing slash (e.g. http://forum.example.com becomes ...comembed/best); normalize the base before joining |
app/assets/javascripts/embed.js:5 | MEDIUM | ✓ Real: a plausible misconfiguration of the embed URL breaks the iframe silently |
post.user may be nil — NoMethodError on author rendering when the post author has been deleted/anonymized; guard with post.user.try(:small_avatar_url) / try(:username) |
app/views/embed/best.html.erb:16 | MEDIUM | ✓ Real: deleted authors are a normal state on Discourse, so the crash is reachable |
Unused feed-modified SHA1 cache key: feed_key is computed but never referenced anywhere, so every hourly run re-parses the full feed; either wire it into a conditional-fetch guard or remove the dead method |
app/jobs/scheduled/poll_feed.rb:20 | MEDIUM | ✓ Real: the intended caching is dead code, so the job is heavier than designed |
URL fallback may pass a non-http (or nil) URL and silently skip the item when both i.link and i.id are blank or non-http; also i.link as a Hash raises TypeError on the regex |
app/jobs/scheduled/poll_feed.rb:33 | MEDIUM | ✓ Real: silent item skipping plus a crash path on hash links |
Spec guards for feed_polling_url and embed_by_username are not isolated: feed_polling_enabled defaults to false, so execute short-circuits before reaching the tested guard |
spec/jobs/poll_feed_spec.rb:18 | MEDIUM | ✓ Real: the tests pass without exercising their own branches — a false sense of coverage |
absolutize_urls corrupts protocol-relative URLs (//host/... gets rewritten to http://host//host/...) and only rewrites root-relative paths, leaving non-leading-slash relative references untouched despite the 'Convert any relative URLs to absolute' claim |
app/models/topic_embed.rb:65 | MEDIUM | ✓ Real: a documented behavior that corrupts protocol-relative URLs — a genuine data-fidelity bug |
Malformed-URL contract inconsistent: a URL passing the ^https?:// prefix but not URI-parseable (e.g. http:// or embedded spaces) raises URI::InvalidURIError instead of returning nil |
app/models/topic_embed.rb:11 | MEDIUM | ✓ Real: the nil-return contract leaks an exception |
Update path assumes embed.post is present (nil if deleted — PostRevisor.new(nil) fails) and never updates the topic title when the feed item's title changes |
app/models/topic_embed.rb:36 | MEDIUM | ✓ Real: deleted embeds crash the update path; changed titles are never reflected |
Incorrect word_count for empty/whitespace-only content: regexp_split_to_array('', ' ') returns one empty element, yielding 1; splitting on literal space also inflates counts for tabs/newlines/multiple spaces |
db/migrate/20131210181901_migrate_word_counts.rb:9 | MEDIUM | ✓ Real: empty posts get word_count 1 and whitespace handling is wrong — a data backfill that miscalculates |
Migration defines only up with no down; rake db:rollback past this backfill raises IrreversibleMigration |
db/migrate/20131210181901_migrate_word_counts.rb:4 | MEDIUM | ✓ Real: a missing down breaks rollback chains |
force: true is destructive and edits an already-applied migration: re-running drops all top_topics data, and the change has no effect on existing DBs where the migration already ran |
db/migrate/20131223171005_create_top_topics.rb:3 | MEDIUM | ✓ Real: editing an applied migration is a data-loss risk in existing installs |
discourse_expires_in 1.minute on the 'loading' branch caches the loading page for anonymous users, while the client reloads after 30s, so embedded clients can be stuck on 'Loading...' for up to ~1 minute after the topic is created |
app/controllers/embed_controller.rb:19 | LOW | ✓ Real: TTL longer than the client's reload interval keeps users on the loading page |
Job enqueue contract relies on inline job execution in the controller spec: it stubs TopicRetriever.new but the controller only enqueues the job; stub Jobs.expects(:enqueue).with(:retrieve_topic, ...) to assert the actual contract |
spec/controllers/embed_controller_spec.rb:31 | LOW | ✓ Real: the spec asserts the wrong layer of the contract |
Loading view reloads forever (document.location.reload() unconditionally every 30s) if the retrieve_topic job never produces an embed; cap the retry count or clear the reload once the topic loads |
app/views/embed/loading.html.erb:8 | LOW | ✓ Real: an infinite reload loop on a permanently failing embed |
embed.permalink locale key is defined but never used, while the post-date is rendered with a hardcoded strftime("%e %b %Y") (best.html.erb:14); remove the dead key or use embed.permalink; prefer l() for localization and avoid %e (non-portable) |
config/locales/server.en.yml:37 | LOW | ✓ Borderline: dead key plus a non-portable format string; a localization hygiene nit |
CGI.unescapeHTML is used without require 'cgi'; it works only via transitive loading. Add an explicit require 'cgi' for robustness |
app/jobs/scheduled/poll_feed.rb:35 | LOW | ✓ Borderline: load-order dependence, currently works by luck |
Update spec cannot distinguish an update from a second create: it only asserts post.cooked matches, which would also pass on a new post; assert TopicEmbed.count stays 1 and the same post is returned |
spec/models/topic_embed_spec.rb:39 | LOW | ✓ Real: a test that cannot fail on the behavior it claims to cover |
| Change-detection sha1 is computed over pre-absolutized contents (including the footer) rather than the stored raw; translation changes can spuriously mark every embed as changed, and the digest may not align with stored content | app/models/topic_embed.rb:16 | LOW | ✓ Real: the change-detection digest is computed over the wrong string |
Digest::SHA1.hexdigest is used without an explicit require 'digest/sha1'; add it to avoid load-order dependence |
app/models/topic_embed.rb:16 | LOW | ✓ Borderline: same load-order class of issue as the cgi require |
create_table ... force: true on a brand-new table drops existing data if the migration is re-run or a table with that name exists; use force: false for the initial create unless destructive idempotency is intended |
db/migrate/20131217174004_create_topic_embeds.rb:3 | LOW | ✓ Real: destructive on re-run; the convention is force: false for new tables |
Schema Information comment not updated for the new cook_method column; regenerating the annotation will keep in-file docs in sync |
app/models/post.rb:503 | LOW | ✓ Borderline: doc-only nit |
cook_method is now a supported PostCreator option but is not documented in the 'Acceptable options' comment block; add it for clarity |
lib/post_creator.rb:216 | LOW | ✓ Borderline: doc-only nit |
Revisor save result is not checked when skip_validations: true (embed import path); a failed save would silently continue into later steps; confirm this is acceptable for the embed path or check the return value |
lib/post_revisor.rb:85 | LOW | ✓ Real: a silent failure path in the embed import |
disable_ddl_transaction! on a pure DML backfill removes transactional guarantees, leaving partial updates on failure; drop it (or document why it is intentionally non-transactional) |
db/migrate/20131210181901_migrate_word_counts.rb:2 | LOW | ✓ Real: a DML backfill without a transaction can leave partial updates |
Inconsistent index-order option style: string form order: 'desc' works in Rails 3.2 but the repo elsewhere uses the hash form; consider aligning for consistency (no functional change) |
db/migrate/20131223171005_create_top_topics.rb:18 | LOW | ✓ Borderline: pure style consistency, no functional impact |
embeddable_host has no format constraint; it is compared byte-for-byte against URI(...).host, so a misconfiguration with scheme/port/trailing slash silently breaks embedding; document the bare-host format or add validation |
config/site_settings.yml:354 | LOW | ✓ Real: a misconfiguration failure mode that is silent until embedding breaks |
Honest Analysis
FriendlyReviewer's full review
As posted on the PR — the complete output from FriendlyReviewer.
Hi there 👋
| 🌥️ Tech |
|---|
| 7 high, 15 medium, 15 low |
The MR implements the embed URL handling and validation feature, spanning the controller → job → retriever pipeline, feed polling, the topic-embed model and migrations, the embed presentation layer (JS/templates/styles), and configuration. The overall architecture and cross-references (routes, settings, locales, schema contracts) are coherent, and several parts are correct (cook_method enum semantics, PostCreator/Revisor integration, dynamic top_topics columns, locale key resolution). However, the review found multiple critical and high-severity issues that block merging: a Ruby syntax error that breaks the primary embed view, a stored-XSS chain through the raw-HTML cooking path with no sanitization, an SSRF vector in the host-only allowlist (scheme/port not validated), a trivially bypassable per-URL throttle that enables job-queue flooding, and a feed job that can crash on nil content. Additional robustness gaps exist across migrations (data-loss risks, irreversible backfills, silent retry loops), specs that test the wrong branches or are not isolated, and presentation-layer origin/handling weaknesses. These need to be addressed before the feature is production-ready.
These might need a close look
- 🔴 lib/topic_retriever.rb (L15)
SSRF:invalid_host?validates only the host component, not scheme or port. An attacker can pointopen(@embed_url)at arbitrary ports (e.g. Redis/SSH) or non-http protocols on the allowlisted host. Validate scheme is http/https and constrain ports. - 🔴 app/controllers/embed_controller.rb (L13)
Rate-limit bypass / job flood:bestenqueuesJobs::retrieve_topicwith no per-IP controller throttle; the Redis throttle is keyed perembed_url, so distinct URLs each get their own key, bypassing the 60s cap and flooding the job queue / triggering repeated outbound fetches. - 🔴 app/controllers/embed_controller.rb (L26)
Referer-based validation is a weak, spoofable security boundary for a network-fetching endpoint: the only gate forX-Frame-Options: ALLOWALLand job enqueue isrequest.referer, which is client-controlled. Also produces false negatives when referer is suppressed. Prefer an origin check or HMAC-signed URL. - 🔴 app/views/embed/best.html.erb (L6)
Template contains a Ruby syntax error:<%- end if %>emitsend ifwith no condition, making thebestview fail with a 500. Must be<%- end %>. - 🔴 app/models/post.rb (L133)
Stored XSS chain:cookreturnsrawverbatim forraw_htmlcook_method, bypassing sanitization; the embed view renders it withraw post.cooked(best.html.erb:18); andtopic_embed.rb:13interpolates an unescaped attacker-controlled URL into the 'Imported from' footer HTML. Sanitize raw HTML (whitelist tags/attributes, strip scripts/event handlers) before storing/rendering. - 🔴 app/jobs/scheduled/poll_feed.rb (L35)
Unhandled nili.contentcrashes the whole feed job:i.content.scrubraisesNoMethodErrorfor items that only have<description>. No rescue around the item loop, so one malformed item skips all remaining. Guard withi.content.to_s.scrubor skip blank content. - 🔴 db/migrate/20131210181901_migrate_word_counts.rb (L7)
Retry loop silently swallows errors and can spin forever under persistent failure; alsorescue PG::Errormay not catchActiveRecord::StatementInvalidon deadlocks in Rails 3.2, defeating the retry. Log/re-raise after the retry budget and rescue the correct error class.
Worth checking
- 🟡 spec/controllers/embed_controller_spec.rb (L21)
Missing controller spec for a mismatched referer host: only the absence of a referer is tested. Add a case where the referer host differs fromembeddable_hostand assert 403 / no job enqueue / no ALLOWALL header. - 🟡 spec/components/topic_retriever_spec.rb (L22)
Retriever spec never exercisesno_throttle(staff bypass) or the scheme/port bypass ofinvalid_host?;retrieved_recently?is always stubbed. Add coverage forno_throttle: trueand for non-http schemes / non-standard ports on the allowlisted host. - 🟡 app/assets/javascripts/embed.js (L13)
Origin validation in the postMessage listener uses a fragile substring check (discourseUrl.indexOf(e.origin)) and never checkse.source. Use exact origin comparison and verifye.source === iframe.contentWindow. - 🟡 app/views/layouts/embed.html.erb (L10)
postMessagetargetOriginis the full referer URL instead of just its origin; browsers may normalize but it is incorrect usage and inconsistent with the parent-side exact-origin check. Pass onlyscheme://host, and guard against blank referer. - 🟡 app/assets/javascripts/embed.js (L5)
iframe src concatenation breaks whendiscourseUrllacks a trailing slash (e.g.http://forum.example.combecomes...comembed/best). Normalize the base before joining. - 🟡 app/views/embed/best.html.erb (L16)
post.usermay be nil →NoMethodErroron author rendering when the post author has been deleted/anonymized. Guard withpost.user.try(:small_avatar_url)/try(:username). - 🟡 app/jobs/scheduled/poll_feed.rb (L20)
Unused feed-modified SHA1 cache key:feed_keyis computed but never referenced anywhere, so every hourly run re-parses the full feed. Either wire it into a conditional-fetch guard or remove the dead method. - 🟡 app/jobs/scheduled/poll_feed.rb (L33)
URL fallback may pass a non-http (or nil) URL and silently skip the item when bothi.linkandi.idare blank or non-http; alsoi.linkas a Hash raisesTypeErroron the regex. Validate/normalize the URL and log when it cannot be resolved. - 🟡 spec/jobs/poll_feed_spec.rb (L18)
Spec guard tests forfeed_polling_urlandembed_by_usernameare not isolated:feed_polling_enableddefaults to false, soexecuteshort-circuits before reaching the tested guard. Stubfeed_polling_enabled?and the other guards so each test exercises its intended branch. - 🟡 app/models/topic_embed.rb (L65)
absolutize_urlscorrupts protocol-relative URLs (//host/...gets rewritten tohttp://host//host/...) and only rewrites root-relative paths, leaving non-leading-slash relative references untouched despite the 'Convert any relative URLs to absolute' claim. Handle protocol-relative URLs as-is and implement full relative resolution or narrow the documented behavior. - 🟡 app/models/topic_embed.rb (L11)
Malformed-URL contract inconsistent: a URL passing the^https?://prefix but not URI-parseable (e.g.http://or embedded spaces) raisesURI::InvalidURIErrorinstead of returning nil. Strengthen validation or rescue the URI error, and add a spec for this case. - 🟡 app/models/topic_embed.rb (L36)
Update path assumesembed.postis present (nil if deleted →PostRevisor.new(nil)fails) and never updates the topic title when the feed item's title changes. Guard for a missing post and update the title when it differs. - 🟡 db/migrate/20131210181901_migrate_word_counts.rb (L9)
Incorrect word_count for empty/whitespace-only content:regexp_split_to_array('', ' ')returns one empty element, yielding 1; splitting on literal space also inflates counts for tabs/newlines/multiple spaces. Use a whitespace class and filter empty strings, guarding the empty case to 0. - 🟡 db/migrate/20131210181901_migrate_word_counts.rb (L4)
Migration defines onlyupwith nodown;rake db:rollbackpast this backfill raisesIrreversibleMigration. Add adownthat resets the backfilled values. - 🟡 db/migrate/20131223171005_create_top_topics.rb (L3)
force: trueis destructive and edits an already-applied migration: re-running drops alltop_topicsdata, and the change has no effect on existing DBs where the migration already ran. Use a new migration orTopTopic.refresh!instead.
Small things (take or leave)
- 🔵 app/controllers/embed_controller.rb (L19)
discourse_expires_in 1.minuteon the 'loading' branch caches the loading page for anonymous users, while the client reloads after 30s, so embedded clients can be stuck on 'Loading...' for up to ~1 minute after the topic is created. Shorten the TTL for the loading branch. - 🔵 spec/controllers/embed_controller_spec.rb (L31)
Job enqueue contract relies on inline job execution in the controller spec: it stubsTopicRetriever.newbut the controller only enqueues the job. StubJobs.expects(:enqueue).with(:retrieve_topic, ...)to assert the actual contract. - 🔵 app/views/embed/loading.html.erb (L8)
Loading view reloads forever (document.location.reload()unconditionally every 30s) if the retrieve_topic job never produces an embed. Cap the retry count or clear the reload once the topic loads. - 🔵 config/locales/server.en.yml (L37)
embed.permalinklocale key is defined but never used, while the post-date is rendered with a hardcodedstrftime("%e %b %Y")(best.html.erb:14). Remove the dead key or useembed.permalink; preferl()for localization and avoid%e(non-portable). - 🔵 app/jobs/scheduled/poll_feed.rb (L35)
CGI.unescapeHTMLis used withoutrequire 'cgi'; it works only via transitive loading. Add an explicitrequire 'cgi'for robustness. - 🔵 spec/models/topic_embed_spec.rb (L39)
Update spec cannot distinguish an update from a second create: it only assertspost.cookedmatches, which would also pass on a new post. AssertTopicEmbed.countstays 1 and the same post is returned. - 🔵 app/models/topic_embed.rb (L16)
Change-detection sha1 is computed over pre-absolutized contents (including the footer) rather than the stored raw; translation changes can spuriously mark every embed as changed, and the digest may not align with stored content. Compute over the value passed to the creator/revisor. - 🔵 app/models/topic_embed.rb (L16)
Digest::SHA1.hexdigestis used without an explicitrequire 'digest/sha1'; add it to avoid load-order dependence. - 🔵 db/migrate/20131217174004_create_topic_embeds.rb (L3)
create_table ... force: trueon a brand-new table drops existing data if the migration is re-run or a table with that name exists. Useforce: falsefor the initial create unless destructive idempotency is intended. - 🔵 app/models/post.rb (L503)
Schema Information comment not updated for the newcook_methodcolumn; regenerating the annotation will keep in-file docs in sync. - 🔵 lib/post_creator.rb (L216)
cook_methodis now a supportedPostCreatoroption but is not documented in the 'Acceptable options' comment block; add it for clarity. - 🔵 lib/post_revisor.rb (L85)
Revisor save result is not checked whenskip_validations: true(embed import path); a failed save would silently continue into later steps. Confirm this is acceptable for the embed path or check the return value. - 🔵 db/migrate/20131210181901_migrate_word_counts.rb (L2)
disable_ddl_transaction!on a pure DML backfill removes transactional guarantees, leaving partial updates on failure. Drop it (or document why it is intentionally non-transactional). - 🔵 db/migrate/20131223171005_create_top_topics.rb (L18)
Inconsistent index-order option style: string formorder: 'desc'works in Rails 3.2 but the repo elsewhere uses the hash form; consider aligning for consistency (no functional change). - 🔵 config/site_settings.yml (L354)
embeddable_hosthas no format constraint; it is compared byte-for-byte againstURI(...).host, so a misconfiguration with scheme/port/trailing slash silently breaks embedding. Document the bare-host format or add validation.
FriendlyReviewer found 6 of the 6 golden bugs of this PR — with one important caveat. Golden #1 (the SSRF on
open(@embed_url)) is caught head-on: the review identifies thatinvalid_host?validates only the host component, not the scheme or port, and names the concrete targets (Redis/SSH ports, non-http protocols on an allowlisted host). Golden #6 (the<%- end if %>syntax error that 500s the primary embed view) is also found, at high severity. Goldens #2, #3 and #4 — theindexOforigin check, thetargetOriginmisuse, and the ALLOWALL/referer gate — are all matched with the same file and line the dataset targets.The honest caveat is golden #5, marked Partial on this page. Its XSS half is fully found: the review chains
raw_htmlcook_method returning raw content, theraw post.cookedrender, and the unescaped attacker-controlled URL interpolated into the 'Imported from' footer. But theNoMethodErrorhalf — a nilcontentsargument crashingTopicEmbed.import— is not found. The review's closest nil-related finding is on the update path (embed.postnil, topic_embed.rb L36), a different call site with a different missing object. Counting that golden as "found" would overstate; counting it as missed would understate. Partial is the fair verdict, and it is the only instance where the run does not map exactly onto the golden set.The volume of extras — 31 findings across the whole pipeline (controller, job, retriever, model, migrations, JS, templates, specs, locale, settings) — is the real story of this page. The strongest are genuinely severe: the rate-limit bypass (the Redis throttle is keyed per
embed_url, so distinct URLs each get their own key and the 60s cap is trivially flooded), thepoll_feedcrash on nili.content(one malformed item kills the entire hourly job), and the migration retry loop that can spin forever while swallowing errors. The weakest are pure hygiene (index-order style, schema-info comments, a dead locale key, two missingrequires) — real but minor, and the page's Legitimate column says so rather than inflating them.Two calibration notes. First, the severity mapping: the dataset's Critical SSRF displays as HIGH per this benchmark's scale, and FriendlyReviewer's own ranking (7 high) puts the SSRF, the referer gate, the stored XSS and the syntax error at the top — matching the dataset's emphasis. Second, several findings cluster on the same lines (three on
poll_feed.rb, two ontopic_embed.rbL16, three onmigrate_word_counts.rb), which is density, not duplication — each targets a distinct defect. The verdict "not production-ready" is unimpeachable here: a syntax error breaking the primary view and an SSRF path alone justify it.