Four API faults found while backfilling coverage, documented but unfixed

[suttondemlong]

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/:id returns 422 Required 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

[suttondemlong]

Branch pushed: fix/api-faults-913, fixing all four. 565 examples, 0 failures; standardrb and erb_lint clean.

No PR yet, deliberately. It is stacked on fix/test-eager-load-coverage (#912), because that is where the specs pinning each of these faults live — the branch flips them from asserting current behaviour to asserting the fix, which is only a coherent diff on top of #912. A PR opened against that base would get zero CI, since all three workflows filter on a main/master base; that is exactly what sank #902. Once #912 merges I will rebase this onto master and open it with full CI.

Two things found while fixing #1 that changed the picture:

1. The client impact was smaller than I wrote, and in the opposite direction. I have corrected the issue body. Native does not reference postables at all, and Ember's ActiveModelSerializer expects id with no _id mapping anywhere in the frontend — so the sideloaded records cannot be matched against post_ids/comment_ids today. This is a repair, not a break.

2. It could not be fixed in isolation. Once the serializers actually ran, two things surfaced: the post factory's encrypted_user_id was the literal "abcd1234", which is not valid ciphertext and had never been decrypted because nothing had ever serialized a post; and serializing each record reads its notifications and reactions, an N+1 that cannot be eager loaded at the call site because the controller queries Postable, which declares no associations. Both are handled on the branch, and both are in the commit message.

Also added a fifth fault to this issue: Usernameable#user_name rescues SymmetricEncryption::CipherError while the error actually raised is OpenSSL::Cipher::CipherError, so that rescue never fires. Left unfixed — it is unrelated to the four here and deserves its own decision about whether to repair the rescue or drop it.

🤖 Generated with Claude Code