Four API faults found while backfilling coverage, documented but unfixed
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 48/100
Direzione di ricerca
Inizia dalle spec fissate in spec/controllers/api/v1/postables_controller_spec.rb, spec/controllers/api/v1/patterns_controller_spec.rb, spec/services/pattern_creator_spec.rb e spec/controllers/api/v1/oracle_requests_controller_spec.rb, quindi esamina i serializer, i controller e il service a cui fanno riferimento. Scegli un problema e conferma il comportamento attuale prima di modificarlo. Il lavoro è completato quando il comportamento selezionato dell'API è corretto e il relativo esempio è aggiornato; esamina l'utilizzo da parte del client prima di modificare le forme delle risposte o la gestione delle date.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Four pre-existing faults found while backfilling coverage in #912. Each is already pinned by a spec on that branch, asserting current behaviour with a comment explaining what correct would look like — so nothing here is silently wrong, and a future fix has to be deliberate rather than accidental. They were left unfixed because each changes an API response shape or is cosmetic, and neither belonged in a PR about coverage.
Listed roughly in order of how much they matter.
1. PostableSerializer returns raw Mongo documents, exposing encrypted_user_id
GET /api/postables sideloads posts, comments, tags, conditions, symptoms and treatments as raw Mongoid documents rather than through the Api::V1 serializers.
"posts": [{
"_id": "6aa0653608be182a8474ee71",
"encrypted_user_id": "abcd1234",
"last_commented": "...", "comments_count": 1, ...
}]
Expected via Api::V1::PostSerializer would be id, type, user_name, priority, comments — and no encrypted_user_id.
Cause. PostableSerializer builds its sideloads with a bare ActiveModel::ArraySerializer:
posts: ActiveModel::ArraySerializer.new(posts, scope: current_user)
ArraySerializer#serializer_for resolves by unqualified class name — it looks for PostSerializer, while this app defines Api::V1::PostSerializer. AMS finds nothing and falls back to DefaultSerializer, which is just object.as_json. Controllers that render json: directly are unaffected, because AMS's controller integration supplies the namespace.
Consequences. The wrong key (_id not id), missing serializer-computed attributes, and encrypted_user_id handed to the client. That value is the Postgres↔Mongo join key; it is ciphertext, not a plaintext id, so this is not a direct disclosure of user ids, but it is internal plumbing that no client needs.
Fix. Pass namespace:. Correction to an earlier version of this issue: I originally wrote that this changes the payload for both clients. That was wrong about native, which does not reference postables at all. And for Ember it is a repair rather than a break — frontend/app/serializers/{post,comment}.js use ActiveModelSerializer, whose primaryKey is id, and there is no _id mapping anywhere in the frontend. Ember Data therefore cannot match the sideloaded records against the post_ids/comment_ids on the fake postable today, so the profile feed is already degraded.
Pinned by spec/controllers/api/v1/postables_controller_spec.rb, "emits sideloaded records as raw documents, not through Api::V1 serializers".
2. PatternsController#show is unreachable
Every request to it fails, one of two ways:
- A plain
GET /api/patterns/:idreturns 422Required parameter missing: pattern - Nesting the id where the action looks for it returns 404, even though the pattern exists
def show
pattern = Pattern.find_by(id: pattern_params[:id])
render json: pattern
end
def pattern_params
params.require(:pattern).permit(:name, :start_at, :end_at, includes: [...])
end
pattern_params requires a pattern key that a normal show request does not send, and does not permit :id, so the lookup runs with nil and Mongoid raises DocumentNotFound. The action also ignores the @pattern that load_and_authorize_resource has already loaded for it.
Fix. render json: @pattern. Low risk — but it turns a 404 into a 200, so it is a behaviour change, and no client appears to call it today.
Pinned by two examples in spec/controllers/api/v1/patterns_controller_spec.rb.
3. PatternCreator silently drops the dates it is given
def initialize(options)
@start_at = options[:start_at]
@end_at = options[:end_at]
...
end
def create
Pattern.create(name: name, includes: includes, encrypted_user_id: encrypted_user_id)
end
start_at and end_at are read, exposed as attr_accessor, and then never passed to Pattern. A range supplied at creation is accepted and discarded, and the caller gets back a persisted pattern with both fields nil.
PatternsController#create permits both, so the API advertises them.
Fix. Either pass them through or stop accepting them. Worth checking whether any client sends them before choosing.
Pinned by spec/services/pattern_creator_spec.rb, "does not persist the start and end dates it was given".
4. Oracle refusal renders an invalid status symbol
render json: {errors: "Unauthorized"}, status: :unauthorised
:unauthorised is the British spelling and is not one of Rack's status symbols, so this raises ArgumentError instead of answering 401. In production ExceptionLogger's rescue_from "Exception" turns that into a 422 quoting the invalid symbol.
The edit is still correctly refused, which is why this has gone unnoticed — only the status code and message are wrong.
Fix. One character: :unauthorized. The most clearly safe of the four; kept here only to keep the set together.
Pinned by spec/controllers/api/v1/oracle_requests_controller_spec.rb, "refuses an edit from somebody without the token, but with the wrong status".
5. Usernameable#user_name rescues an exception class that is never raised
Found while fixing #1, and not fixed — it is unrelated to the four above.
def user_name
Profile.select(:screen_name).find_by!(user_id: SymmetricEncryption.decrypt(encrypted_user_id)).screen_name
rescue SymmetricEncryption::CipherError
""
end
The rescue is meant to degrade to an empty name when encrypted_user_id will not decrypt. It never fires, because the error actually raised is OpenSSL::Cipher::CipherError, which is not a SymmetricEncryption::CipherError:
SymmetricEncryption::CipherError defined? "constant"
raised: OpenSSL::Cipher::CipherError
is a SymmetricEncryption::CipherError? false
Reachable in practice only if a stored id stops decrypting — a key rotation, or data written outside the app — so this is defensive code that does not defend. Note that find_by! is also unrescued, so a decryptable id whose profile has since been deleted raises RecordNotFound through the serializer.
Low priority, but worth either fixing the rescue or deleting it, rather than leaving something that reads as handled and is not.
Ordering note
The specs in #912 assert current behaviour. Fixing any of these will turn the corresponding example red, which is intended — the failure names the fault and points at the fix. Whoever takes one of these should update that example in the same commit.
🤖 Generated with Claude Code
- Lingua principale
- Ruby
- Stelle
- 50
- Fork
- 21
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
- Include un Dockerfile o un file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di rubyforgood/Flaredown
-
Coverage is measured differently locally and on CI, hiding 68 untested app filesForse già presa @suttondemlong l’ha presa 47 giorni fa. Apertalanguage:ruby type:bug
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
rubyforgood/Flaredown#911 · 1 commento ·
-
type:infrastructure
Difficoltà 3/5 1-2 giorni Idoneità per principianti 76/100
rubyforgood/Flaredown#904 ·
-
Upgrade Rails 8.0 -> 8.1Apertatype:infrastructure
Difficoltà 4/5 3-5 giorni Idoneità per principianti 45/100
rubyforgood/Flaredown#888 ·
-
type:infrastructure
Difficoltà 4/5 3-5 giorni Idoneità per principianti 38/100
rubyforgood/Flaredown#887 ·
-
type:infrastructure
Difficoltà 4/5 3-5 giorni Idoneità per principianti 48/100
rubyforgood/Flaredown#886 ·
Tutte le issue di rubyforgood/Flaredown
Issue simili
-
Liens externes cassésAperta
Difficoltà 1/5 1-3 ore Idoneità per principianti 66/100
betagouv/conseillers-entreprises#4770 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 85/100
seek4science/seek#2762 ·
I maintainer di solito rispondono entro 5 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 67/100
Freika/dawarich#3910 · 3 reazioni ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
rubyevents/rubyevents#2277 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 1/5 1-3 ore Idoneità per principianti 92/100
I maintainer di solito rispondono entro 2 giorni