Discourse/discourse
Discourse/discourse
0
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2026-08-08 17:53:55 +08:00
Code Packages Activity Actions
discourse/plugins/discourse-github/spec/lib/github_linkback_spec.rb
Régis Hanol 36a8a51ef0
 FEATURE: Route all GitHub API requests through one rate-limited client (#40637) 
GitHub oneboxes and the discourse-github plugin talked to GitHub's REST
and
GraphQL API with no rate-limit awareness. On busy instances this
exhausted
GitHub's limits (60 requests/hour unauthenticated, 5000 authenticated),
and
because there was no backoff every render kept hitting GitHub and
re-failing
-- which GitHub's docs warn can get an integration banned. The recently
added PR-status onebox multiplied the number of calls and made it far
worse.

GitHub access was also fragmented: the core onebox engines used OpenURI,
the
discourse-github plugin used Octokit, and the discourse-ai bot tools
used
FinalDestination::HTTP -- three HTTP stacks, three tokens, and
inconsistent
(or entirely missing) error and rate-limit handling.

This introduces a single client, Discourse::GithubApi, that every GitHub
data-API request now flows through. It is built on Faraday with the
SSRF-safe
FinalDestination adapter and:

- authenticates per token (Bearer) and returns plain string-keyed Hashes
(get/post) or raw bodies (raw_get) -- one response shape, no
Octokit/Sawyer
- only ever sends the access token to api.github.com and
  raw.githubusercontent.com, rejecting any other absolute URL, so a
  user-derived path can never leak a token to an arbitrary host
- backs off on rate limits both reactively (403/429) and proactively
(when
X-RateLimit-Remaining hits 0), honouring Retry-After /
X-RateLimit-Reset,
via a shared Redis flag (GithubRateLimit) keyed per token so each
token's
  budget and the shared unauthenticated/IP budget back off independently
- short-circuits while backing off without ever sleeping, so onebox
rendering
  and post baking degrade to a plain link instead of blocking a request
- caches ETags and sends If-None-Match, so unchanged resources return
304s
  that do not count against the rate limit

Every caller was moved onto it:

- the 6 core GitHub onebox engines, via a slimmed
Onebox::Mixins::GithubApi
adapter that keeps their public methods and translates client errors
back
to the OpenURI::HTTPError vocabulary they already rescue (engines
unchanged)
- the github_blob raw.githubusercontent.com fetch
- the discourse-github plugin (badges, linkback, permalinks, token
validator),
which no longer uses the octokit and sawyer gems (they stay in the
Gemfile for
the discourse-code-review official plugin, which still depends on them)
- the discourse-ai bot's GitHub tools (search code, diff, file content,
  search files)

Also adds a GithubOneboxBackoff admin problem check that surfaces while
one of
the onebox token identities is backing off -- scoped to the tokens
resolved by
Onebox::GithubAccess (each configured github_onebox_access_tokens entry
plus the
unauthenticated client) so a backoff on the AI bot or linkback token is
not
misattributed to onebox. Its message points admins at the relevant
setting with
the {{setting:...}} link marker, which problem-check messages now expand
too.
Onebox token resolution is centralised in Onebox::GithubAccess, and the
onebox
cache TTL for transient GitHub failures is shortened so they recover
quickly.

GitHub OAuth login, theme git-clone, the inbound webhook, and the
Oneboxer
FinalDestination URL-resolution special-cases for github.com are
intentionally
out of scope -- they are different concerns, not the rate-limited data
API.
2026-06-15 10:59:10 +02:00

490 lines
18 KiB
Ruby
Vendored
Raw Permalink Blame History

# frozen_string_literal: true
describe GithubLinkback do
let(:github_commit_link) do
"https://github.com/discourse/discourse/commit/76981605fa10975e2e7af457e2f6a31909e0c811"
end
let(:github_commit_link_with_anchor) { "#{github_commit_link}#anchor" }
let(:github_issue_link) { "https://github.com/discourse/discourse/issues/123" }
let(:github_pr_link) { "https://github.com/discourse/discourse/pull/701" }
let(:github_pr_files_link) { "https://github.com/discourse/discourse/pull/701/files" }
let(:github_pr_link_wildcard) { "https://github.com/discourse/discourse-github-linkback/pull/3" }
let(:post) { Fabricate(:post, raw: <<~RAW) }
cool post
#{github_commit_link}
https://eviltrout.com/not-a-gh-link
#{github_commit_link}
#{github_commit_link_with_anchor}
https://github.com/eviltrout/tis-100/commit/e22b23f354e3a1c31bc7ad37a6a309fd6daf18f4
#{github_issue_link}
#{github_pr_link}
#{github_pr_files_link}
i have no idea what i'm linking back to
#{github_pr_link_wildcard}
end_of_transmission
RAW
before { enable_current_plugin }
describe "#should_enqueue?" do
let(:post_without_link) { Fabricate.build(:post, raw: "Hello github!") }
let(:small_action_post) do
Fabricate.build(
:post,
post_type: Post.types[:small_action],
raw:
"https://github.com/discourse/discourse/commit/5be9bee2307dd517c26e6ef269471aceba5d5acf",
)
end
let(:post_with_link) do
Fabricate.build(
:post,
raw:
"https://github.com/discourse/discourse/commit/5be9bee2307dd517c26e6ef269471aceba5d5acf",
)
end
it "returns false when the feature is disabled" do
SiteSetting.github_linkback_enabled = false
expect(GithubLinkback.new(post_with_link).should_enqueue?).to eq(false)
end
it "returns false without a post" do
SiteSetting.github_linkback_enabled = true
expect(GithubLinkback.new(nil).should_enqueue?).to eq(false)
end
it "returns false if the post is not a regular post" do
SiteSetting.github_linkback_enabled = true
expect(GithubLinkback.new(small_action_post).should_enqueue?).to eq(false)
end
it "returns false when the post doesn't have the `github.com` in it" do
SiteSetting.github_linkback_enabled = true
expect(GithubLinkback.new(post_without_link).should_enqueue?).to eq(false)
end
it "returns true when the feature is enabled" do
SiteSetting.github_linkback_enabled = true
expect(GithubLinkback.new(post_with_link).should_enqueue?).to eq(true)
end
describe "private_message" do
it "doesn't enqueue private messages" do
SiteSetting.github_linkback_enabled = true
private_topic = Fabricate(:private_message_topic)
private_post =
Fabricate(
:post,
topic: private_topic,
raw: "this post http://github.com should not enqueue",
)
expect(GithubLinkback.new(private_post).should_enqueue?).to eq(false)
end
end
describe "unlisted topics" do
it "doesn't enqueue unlisted topics" do
SiteSetting.github_linkback_enabled = true
unlisted_topic = Fabricate(:topic, visible: false)
unlisted_post =
Fabricate(
:post,
topic: unlisted_topic,
raw: "this post http://github.com should not enqueue",
)
expect(GithubLinkback.new(unlisted_post).should_enqueue?).to eq(false)
end
end
describe "ignored categories" do
fab!(:category)
fab!(:post_in_category) do
topic = Fabricate(:topic, category:)
Fabricate(:post, topic:, raw: "https://github.com/discourse/discourse/commit/abc123")
end
before { SiteSetting.github_linkback_enabled = true }
it "doesn't enqueue posts in ignored categories" do
SiteSetting.github_linkback_ignored_categories = category.id.to_s
expect(GithubLinkback.new(post_in_category).should_enqueue?).to eq(false)
end
it "enqueues posts in non-ignored categories" do
other_category = Fabricate(:category)
SiteSetting.github_linkback_ignored_categories = other_category.id.to_s
expect(GithubLinkback.new(post_in_category).should_enqueue?).to eq(true)
end
it "handles multiple ignored categories" do
other_category = Fabricate(:category)
SiteSetting.github_linkback_ignored_categories = "#{category.id}|#{other_category.id}"
expect(GithubLinkback.new(post_in_category).should_enqueue?).to eq(false)
end
it "enqueues when ignored categories setting is empty" do
SiteSetting.github_linkback_ignored_categories = ""
expect(GithubLinkback.new(post_in_category).should_enqueue?).to eq(true)
end
it "enqueues posts without a category" do
SiteSetting.github_linkback_ignored_categories = category.id.to_s
uncategorized_post =
Fabricate(:post, raw: "https://github.com/discourse/discourse/commit/abc123")
expect(GithubLinkback.new(uncategorized_post).should_enqueue?).to eq(true)
end
end
end
describe "#github_links" do
it "returns an empty array with no projects" do
SiteSetting.github_linkback_projects = ""
links = GithubLinkback.new(post).github_links
expect(links).to eq([])
end
it "doesn't return links that have already been posted" do
SiteSetting.github_linkback_projects =
"discourse/discourse|eviltrout/ember-performance|discourse/*"
post.custom_fields[GithubLinkback.field_for(github_commit_link)] = "true"
post.custom_fields[GithubLinkback.field_for(github_issue_link)] = "true"
post.custom_fields[GithubLinkback.field_for(github_pr_link)] = "true"
post.custom_fields[GithubLinkback.field_for(github_pr_link_wildcard)] = "true"
post.save_custom_fields
links = GithubLinkback.new(post).github_links
expect(links.size).to eq(0)
end
it "should return the urls for the selected projects" do
SiteSetting.github_linkback_projects =
"discourse/discourse|eviltrout/ember-performance|discourse/*"
links = GithubLinkback.new(post).github_links
expect(links.size).to eq(4)
expect(links[0].url).to eq(github_commit_link)
expect(links[0].project).to eq("discourse/discourse")
expect(links[0].sha).to eq("76981605fa10975e2e7af457e2f6a31909e0c811")
expect(links[0].type).to eq(:commit)
expect(links[1].url).to eq(github_issue_link)
expect(links[1].project).to eq("discourse/discourse")
expect(links[1].issue_number).to eq(123)
expect(links[1].type).to eq(:issue)
expect(links[2].url).to eq(github_pr_link)
expect(links[2].project).to eq("discourse/discourse")
expect(links[2].pr_number).to eq(701)
expect(links[2].type).to eq(:pr)
expect(links[3].url).to eq(github_pr_link_wildcard)
expect(links[3].project).to eq("discourse/discourse-github-linkback")
expect(links[3].pr_number).to eq(3)
expect(links[3].type).to eq(:pr)
end
end
describe "#create" do
let(:github_api_headers) { { "Authorization" => "Bearer abcdef" } }
before { SiteSetting.github_linkback_projects = "discourse/discourse|discourse/*" }
it "returns an empty array without an access token" do
expect(GithubLinkback.new(post).create).to be_blank
end
context "with an access token" do
let(:empty_json_response) do
{ status: 200, body: "[]", headers: { "Content-Type" => "application/json" } }
end
let(:empty_body_json_response) do
{ status: 200, body: '{"body":""}', headers: { "Content-Type" => "application/json" } }
end
before do
SiteSetting.github_linkback_access_token = "abcdef"
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/issues/123/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/issues/701/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse-github-linkback/issues/3/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
stub_request(:get, %r{https://api\.github\.com/repos/.+/commits/.+/comments}).to_return(
empty_json_response,
)
stub_request(:get, %r{https://api\.github\.com/repos/.+/issues/\d+$}).to_return(
empty_body_json_response,
)
stub_request(:get, %r{https://api\.github\.com/repos/.+/issues/\d+/comments}).to_return(
empty_json_response,
)
end
it "returns the URLs it linked to and creates custom fields" do
links = GithubLinkback.new(post).create
expect(links.size).to eq(4)
expect(links[0].url).to eq(github_commit_link)
field = GithubLinkback.field_for(github_commit_link)
expect(post.custom_fields[field]).to be_present
expect(links[1].url).to eq(github_issue_link)
field = GithubLinkback.field_for(github_issue_link)
expect(post.custom_fields[field]).to be_present
expect(links[2].url).to eq(github_pr_link)
field = GithubLinkback.field_for(github_pr_link)
expect(post.custom_fields[field]).to be_present
expect(links[3].url).to eq(github_pr_link_wildcard)
field = GithubLinkback.field_for(github_pr_link_wildcard)
expect(post.custom_fields[field]).to be_present
end
it "should create linkback for <= SiteSetting.github_linkback_maximum_links urls" do
SiteSetting.github_linkback_maximum_links = 2
post = Fabricate(:post, raw: "#{github_pr_link} #{github_issue_link}")
links = GithubLinkback.new(post).create
expect(links.size).to eq(2)
end
it "should skip linkback for > SiteSetting.github_linkback_maximum_links urls" do
SiteSetting.github_linkback_maximum_links = 1
post = Fabricate(:post, raw: "#{github_pr_link} #{github_issue_link}")
links = GithubLinkback.new(post).create
expect(links.size).to eq(0)
end
it "should create linkback for <= SiteSetting.github_linkback_maximum_links unique urls" do
SiteSetting.github_linkback_maximum_links = 1
post = Fabricate(:post, raw: "#{github_pr_link} #{github_pr_link} #{github_pr_link}")
links = GithubLinkback.new(post).create
expect(links.size).to eq(1)
end
end
context "when topic is already linked on GitHub" do
let(:pr_post) { Fabricate(:post, raw: github_pr_link) }
let(:issue_post) { Fabricate(:post, raw: github_issue_link) }
let(:commit_post) { Fabricate(:post, raw: github_commit_link) }
before { SiteSetting.github_linkback_access_token = "abcdef" }
it "skips linkback when the PR description already contains a link to the topic" do
topic_url = "#{Discourse.base_url}/t/#{pr_post.topic.slug}/#{pr_post.topic_id}"
stub_request(:get, "https://api.github.com/repos/discourse/discourse/issues/701").with(
headers: github_api_headers,
).to_return(
status: 200,
body: { body: "This PR implements #{topic_url}" }.to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/issues/701/comments?per_page=100",
).with(headers: github_api_headers).to_return(
status: 200,
body: [].to_json,
headers: {
"Content-Type" => "application/json",
},
)
links = GithubLinkback.new(pr_post).create
expect(links.size).to eq(1)
expect(
a_request(:post, "https://api.github.com/repos/discourse/discourse/issues/701/comments"),
).not_to have_been_made
end
it "skips linkback when an existing comment contains a link to the topic" do
topic_url = "#{Discourse.base_url}/t/#{issue_post.topic_id}/3"
stub_request(:get, "https://api.github.com/repos/discourse/discourse/issues/123").with(
headers: github_api_headers,
).to_return(
status: 200,
body: { body: "Some unrelated description" }.to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/issues/123/comments?per_page=100",
).with(headers: github_api_headers).to_return(
status: 200,
body: [{ body: "Already discussed at #{topic_url}" }].to_json,
headers: {
"Content-Type" => "application/json",
},
)
links = GithubLinkback.new(issue_post).create
expect(links.size).to eq(1)
expect(
a_request(:post, "https://api.github.com/repos/discourse/discourse/issues/123/comments"),
).not_to have_been_made
end
it "skips linkback for commits when an existing comment links to the topic" do
topic_url = "#{Discourse.base_url}/t/#{commit_post.topic_id}"
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments?per_page=100",
).with(headers: github_api_headers).to_return(
status: 200,
body: [{ body: "Discussed in #{topic_url}" }].to_json,
headers: {
"Content-Type" => "application/json",
},
)
links = GithubLinkback.new(commit_post).create
expect(links.size).to eq(1)
expect(
a_request(
:post,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments",
),
).not_to have_been_made
end
end
context "when topic is not already linked on GitHub" do
let(:pr_post) { Fabricate(:post, raw: github_pr_link) }
let(:commit_post) { Fabricate(:post, raw: github_commit_link) }
before { SiteSetting.github_linkback_access_token = "abcdef" }
it "posts linkback when existing links point to a different topic" do
different_topic_url = "#{Discourse.base_url}/t/some-other-topic/999999"
stub_request(:get, "https://api.github.com/repos/discourse/discourse/issues/701").with(
headers: github_api_headers,
).to_return(
status: 200,
body: { body: "See #{different_topic_url}" }.to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/issues/701/comments?per_page=100",
).with(headers: github_api_headers).to_return(
status: 200,
body: [].to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/issues/701/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
links = GithubLinkback.new(pr_post).create
expect(links.size).to eq(1)
expect(
a_request(:post, "https://api.github.com/repos/discourse/discourse/issues/701/comments"),
).to have_been_made.once
end
it "posts linkback when the GitHub API check fails (fail open)" do
stub_request(:get, "https://api.github.com/repos/discourse/discourse/issues/701").with(
headers: github_api_headers,
).to_return(status: 404, body: "", headers: {})
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/issues/701/comments?per_page=100",
).with(headers: github_api_headers).to_return(status: 404, body: "", headers: {})
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/issues/701/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
links = GithubLinkback.new(pr_post).create
expect(links.size).to eq(1)
expect(
a_request(:post, "https://api.github.com/repos/discourse/discourse/issues/701/comments"),
).to have_been_made.once
end
it "posts linkback for commits when no existing comment links to the topic" do
stub_request(
:get,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments?per_page=100",
).with(headers: github_api_headers).to_return(
status: 200,
body: [{ body: "Some unrelated comment" }].to_json,
headers: {
"Content-Type" => "application/json",
},
)
stub_request(
:post,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments",
).with(headers: github_api_headers).to_return(status: 200, body: "", headers: {})
links = GithubLinkback.new(commit_post).create
expect(links.size).to eq(1)
expect(
a_request(
:post,
"https://api.github.com/repos/discourse/discourse/commits/76981605fa10975e2e7af457e2f6a31909e0c811/comments",
),
).to have_been_made.once
end
end
end
end
View git blame Copy permalink