Skip to content

Escape values interpolated into graphql queries - #673

Open
Thayorns wants to merge 2 commits into
zerocracy:masterfrom
Thayorns:660
Open

Escape values interpolated into graphql queries#673
Thayorns wants to merge 2 commits into
zerocracy:masterfrom
Thayorns:660

Conversation

@Thayorns

@Thayorns Thayorns commented Sep 2, 2026

Copy link
Copy Markdown

Every query in lib/fbe/github_graph.rb dropped its arguments straight into the
query text, so an owner, a repository name, a branch or a pagination cursor
carrying a double quote silently rewrote the query instead of failing. Feeding
a") { id } repository(owner: "x as an owner turned one repository selection
into two fields, and graphql-ruby accepted it, so the caller got an answer about
something it never asked for.

Every such value now goes through a small Fbe::Graph::Literal object that
renders it as a quoted and escaped GraphQL string literal. The call sites lose
their hand-written quotes and read owner: #{Literal.new(owner)}, which keeps
the query text fixed no matter what the value carries. This covers owner, name,
branch, node id, the after: cursor in all four paginated queries, and the two
search(query:) strings in total_issues_created.

Two tests drive it: one feeds resolved_conversations an owner that tries to
open a second repository field, one feeds pull_requests_with_reviews a
cursor that tries to add an argument. Both assert the value arrives as a single
escaped literal, and both failed before the change.

allow_dynamic_queries stays on, since the multi-repo and multi-pull queries are
still assembled at runtime.

Closes #660

@Thayorns

Thayorns commented Sep 2, 2026

Copy link
Copy Markdown
Author

@yegor256 happy to clarify anything here, just say the word.

@Thayorns

Thayorns commented Sep 2, 2026

Copy link
Copy Markdown
Author

@yegor256 fixed here, take a look please

yegor256 commented Sep 3, 2026

Copy link
Copy Markdown
Member

This is the largest of the github_graph.rb changes and overlaps nearly every other open PR touching that file (#674, #669, #667, #670, #678, #671). Needs a human to pick a merge order or reconcile rather than merging blind. Assigning to @yegor256.


Generated by Claude Code

@Thayorns Thayorns changed the title fix(#660): escape values interpolated into graphql queries Escape values interpolated into graphql queries Sep 3, 2026
@Thayorns

Thayorns commented Sep 3, 2026

Copy link
Copy Markdown
Author

@yegor256 take a look please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Repository and branch names are interpolated into GraphQL queries unescaped in lib/fbe/github_graph.rb

2 participants