Sorted by the Word ‘difficulty’

I have written this line more times than I could count:

Challenge.includes(:event).where(category: params[:category]).order(params[:sort])

This series keeps twenty years of Rails reflexes switched on inside a Hanami 3 application, a small CTF challenge tracker, and records each place where they stop matching what the framework does. The post on escaping started on this screen, the list of challenges with its filters. This one comes from earlier the same day, when the sort did not sort.

The half that felt like home

The filters went in first, and there is nothing to report about them, which deserves a sentence of its own in a series about friction. In Hanami the query belongs to the repo rather than the action, but building it uses the same moves. where and order chain onto a relation exactly as they do on an Active Record scope:

def all_with_event(category: nil, solved: nil, sort: nil)
  listing = challenges.combine(:event)
  listing = listing.where(category: category) if category
  listing = listing.where(solved: solved) unless solved.nil?
  listing = listing.order(sort) if sort
  listing.to_a
end

That is the first version of the method. The category filter and the solved filter both worked on the first try. So did the sort, according to its spec:

it "orders by the requested column" do
  repo.create(attributes.merge(name: "Hard", difficulty: 5))
  repo.create(attributes.merge(name: "Easy", difficulty: 1))
  repo.create(attributes.merge(name: "Medium", difficulty: 3))

  listed = repo.all_with_event(sort: :difficulty)

  expect(listed.map(&:name)).to eq(%w[Easy Medium Hard])
end

Green. On the page, though, sorting by difficulty changed nothing: the list stayed in the order the rows had been inserted.

A spec that spoke Ruby

The argument in that spec is :difficulty. I wrote a Symbol because I was writing Ruby, and in Ruby a column name is a Symbol. A browser does not send Symbols. Here is what the action hands the repo, recorded by injecting a stand-in repo that prints its arguments, for two query strings against the finished action:

?solved=true&sort=difficulty
  repo receives {category: nil, solved: true, sort: "difficulty"}
  status 200
?sort=nope
  status 422

solved arrives as true, and sort arrives as a String. (The 422 for nope comes from the fix further down.) My spec had tested a value the application never receives. It proved the repo could sort by a Symbol, and said nothing about the only caller that mattered.

To check that I was not telling myself a tidy story after the fact, I put the first version of the repo back into an isolated copy of the app, alongside the two specs that came later, one passing a String to the repo and one going through HTTP. The original spec passes, and the other two fail:

  1) CtfTracker::Repos::ChallengeRepo#all_with_event, narrowed orders by a column name that arrived as a string
     Failure/Error: expect(listed.map(&:name)).to eq(%w[Easy Medium Hard])

       expected: ["Easy", "Medium", "Hard"]
            got: ["Hard", "Easy", "Medium"]

  2) GET /challenges orders the list by the column asked for in the query string
     Failure/Error: expect(last_response.body.index("Easy")).to be < last_response.body.index("Hard")

       expected: < 1043
            got:   1212

3 examples, 2 failures

(Output trimmed to the failures and the summary.)

A word, not a column

What the repo does with that String is the actual surprise. I ran the same relation through a handful of sort values on the app’s own database, three rows inserted as Hard, Easy, Medium:

:difficulty                          -> ORDER BY `difficulty`  ["Easy", "Medium", "Hard"]
"difficulty"                         -> ORDER BY 'difficulty'  ["Hard", "Easy", "Medium"]
"difficulty desc"                    -> ORDER BY 'difficulty desc'  ["Hard", "Easy", "Medium"]
:nope                                -> Sequel::DatabaseError
"nope"                               -> ORDER BY 'nope'  ["Hard", "Easy", "Medium"]
"difficulty; drop table challenges"  -> ORDER BY 'difficulty; drop table challenges'  ["Hard", "Easy", "Medium"]
"1=1"                                -> ORDER BY '1=1'  ["Hard", "Easy", "Medium"]
table still there: 3 rows

Look at the quotes. A Symbol becomes an identifier in backticks, a column. A String becomes a string literal in single quotes, a value. So ORDER BY 'difficulty' sorts every row by the same constant word, every row ties, and the database returns them in whatever order it likes. Nothing raises, and the only trace is a SQL line in the debug log. To anyone using the page, the sort link is simply broken.

This is not a quirk of ROM, which hands order to Sequel underneath. Sequel’s security guide says it plainly: it “treats ruby symbols as SQL identifiers, and ruby strings as SQL strings”, and since Sequel 5 it no longer turns plain strings into raw SQL in ordinary code.

The last lines of that output matter just as much. The String with drop table in it came back as a quoted value, and the table was still there afterwards. There is no injection through order here, for exactly the reason there is no sorting: Sequel never reads a String as SQL. The safety and the silence come from one design decision.

The same line in Rails

My belief about Rails was the reverse: that order("difficulty") works there, because a String is a SQL fragment, and that this is exactly why passing params[:sort] to it is dangerous. Memory is not evidence, so I ran the same values against Active Record 8.1.4.1 on SQLite:

:difficulty                          -> ORDER BY "challenges"."difficulty" ASC  ["Easy", "Medium", "Hard"]
"difficulty"                         -> ORDER BY difficulty  ["Easy", "Medium", "Hard"]
"difficulty desc"                    -> ORDER BY difficulty desc  ["Hard", "Medium", "Easy"]
:nope                                -> ORDER BY "nope" ASC  ["Hard", "Easy", "Medium"]
"nope"                               -> ActiveRecord::StatementInvalid
"difficulty; drop table challenges"  -> ActiveRecord::UnknownAttributeReference
"1=1"                                -> ActiveRecord::UnknownAttributeReference
"(select 1)"                         -> ActiveRecord::UnknownAttributeReference
"lower(name)"                        -> ORDER BY lower(name)  ["Easy", "Hard", "Medium"]
table still there: 3 rows

Half of my memory held. The String goes into the SQL as written, so the Rails version of my line sorts correctly from a query string. The other half was out of date. Rails no longer lets any String through: the 5.2 changelog deprecated raw SQL in query methods and announced UnknownAttributeReference for 6.0. What decides is a regular expression in the SQLite adapter, column_name_with_order_matcher. It accepts a column name, optionally qualified by its table, or a function call wrapping one column or nothing, each followed by an optional collation and an optional ASC or DESC, and a comma-separated list of those. Anything shaped differently raises, unless you wrap it in Arel.sql to say you trust it.

So Rails guesses what a String is from its shape, and Sequel decides from its class. Both answers are defensible. Rails’ guess is generous: lower(name) passes, which is a function call chosen by whoever edited the URL, harmless here and still not something I meant to offer. And "nope" is a 500 in Rails and a silent no-op in Sequel, which means the naive line is wrong in both frameworks, for different reasons.

One row in that table needs a footnote. In Rails, :nope returned rows instead of failing. That is SQLite rather than Rails: SQLite reads a double-quoted name that matches no column as a string literal, a compatibility choice its own documentation calls a misfeature. Sequel quotes identifiers with backticks on SQLite, so the same mistake is an error there. Both runs used SQLite, and I have not checked this row on another database.

Solved got a type, sort got a list

The solved filter has the same shape as the sort: a query string delivers "true", and the repo wants true. It worked on the first try because the action’s params schema has a type for it, and coerces the value at the edge. The output above shows it: solved: true, a real boolean, by the time the repo sees it.

There is no type for “a column of this table”. filled(:string) accepts "difficulty" and "nope" with the same enthusiasm. A column name needs a list of allowed values, which is a different kind of statement from a type. So the repo now names its sortable columns and converts the String itself, but only into one of them:

# Sequel orders by a Symbol as a column and by a String as a literal
# value, so a sort that arrives from a query string orders by nothing at
# all, in silence. Naming the columns we accept settles that and keeps an
# unknown one from reaching the database as an identifier.
SORTABLE = %i[name category difficulty points created_at].freeze

def sortable(sort)
  column = sort.to_sym
  return column if SORTABLE.include?(column)

  raise ArgumentError, "cannot sort challenges by #{sort.inspect}"
end

The list matters as much as the conversion. A bare to_sym fixes the sort and opens the other door: :nope reaches SQLite as an identifier and comes back as a Sequel::DatabaseError, a 500 for someone who edited a URL. The ArgumentError stops it before the database, but it is still a programming error, not an answer for a visitor. So the action’s params schema offers the same list:

params do
  optional(:category).filled(:string)
  optional(:solved).filled(:bool)
  optional(:sort).filled(included_in?: Repos::ChallengeRepo::SORTABLE.map(&:to_s))
end

An edited sort is now a 422, and the repo’s refusal is a guard that should never fire in practice. The template builds its <select> from the same constant, through a sorts exposure, rather than keeping a third copy of the list.

In Rails I would have put that list in a constant on the model, and let the controller and the view read it from there. Hanami has no model to hang it on, and the repo turns out to be the same answer under another name: it owns the query, so it owns the list. Three layers need to agree about one list, and the arrangement that stays correct is two of them asking the third.

What I left undone

The form does not show the current filters after a submit, so filtering by pwn and then picking a sort drops the category. That is an ordinary omission, nothing to do with Hanami, and I would rather say so than hide it behind a screenshot.

The spec I started with is still in the suite, next to the two that would have caught this: one passing the String the repo really receives, one going through HTTP like a browser. The lesson I took is narrower than “test more”. A spec is written in Ruby and an application receives text. By writing :difficulty in that spec, I had done the conversion the application never did, and then asked the application to agree with me.

The filters, the sort and the specs are one commit in the companion app, tagged 05-symbol-or-string.