Four API faults found while backfilling coverage, documented but unfixed
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 48/100
Línea de trabajo
Empieza por las specs fijadas en spec/controllers/api/v1/postables_controller_spec.rb, spec/controllers/api/v1/patterns_controller_spec.rb, spec/services/pattern_creator_spec.rb y spec/controllers/api/v1/oracle_requests_controller_spec.rb; después, inspecciona los serializers, controllers y el service referenciados. Elige un fallo y confirma su comportamiento actual antes de cambiarlo. La tarea estará terminada cuando se haya corregido el comportamiento seleccionado de la API y actualizado su ejemplo correspondiente; revisa el uso por parte del cliente antes de cambiar las formas de respuesta o el manejo de fechas.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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
- Lenguaje dominante
- Ruby
- Estrellas
- 50
- Forks
- 21
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Preparar el entorno
- Incluye un Dockerfile o un archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de rubyforgood/Flaredown
-
Coverage is measured differently locally and on CI, hiding 68 untested app filesPosiblemente ocupada @suttondemlong la tomó hace 47 días. Abiertolanguage:ruby type:bug
Dificultad 5/5 Más de una semana Aptitud para principiantes 35/100
rubyforgood/Flaredown#911 · 1 comentario ·
-
Enable the Mongoid 9 defaultsAbiertotype:infrastructure
Dificultad 3/5 1-2 días Aptitud para principiantes 76/100
rubyforgood/Flaredown#904 ·
-
Upgrade Rails 8.0 -> 8.1Abiertotype:infrastructure
Dificultad 4/5 3-5 días Aptitud para principiantes 45/100
rubyforgood/Flaredown#888 ·
-
type:infrastructure
Dificultad 4/5 3-5 días Aptitud para principiantes 38/100
rubyforgood/Flaredown#887 ·
-
type:infrastructure
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
rubyforgood/Flaredown#886 ·
Todos los issues de rubyforgood/Flaredown
Issues similares
-
Liens externes cassésAbierto
Dificultad 1/5 1-3 horas Aptitud para principiantes 66/100
betagouv/conseillers-entreprises#4770 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 85/100
seek4science/seek#2762 ·
Los mantenedores suelen responder en 5 días
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
rubyevents/rubyevents#2277 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 1/5 1-3 horas Aptitud para principiantes 92/100
Los mantenedores suelen responder en 2 días