Cover image credit: Photo by Nick Fewings on Unsplash
This article explores how to ensure your APIs are safe. Specifically, we address a critical issue that arises when incorrect database actions are coupled with inappropriate HTTP request verbs creating an “unsafe API method.”
“Request methods are considered safe if their defined semantics are essentially read-only; i.e., the client does not request, and does not expect, any state change on the origin server as a result of applying a safe method to a target resource.” (The RFC Documentation)
GET and HEAD requests should be idempotent, which means they shouldn’t cause any side effects on the resources on the server. For most API endpoints, this means that calling them once will return the same values as calling them multiple times. For some APIs (like one that can GET the current time, or look up a stock’s latest market price) it might return different data each time, but still be idempotent as long as it isn’t the API that causes those changes in the server’s data values.
If a supposedly safe API breaks the idempotency rule, it can cause a lot of issues for a system. Clients of the API need to be 100% sure that their read-only calls truly are read-only. After all, nobody would trust an ATM if the act of checking their bank balance caused it to drop in value each time!
Code reviewers are doing their job well when they check read-only APIs to be safe. However, they can sometimes miss things, especially with library-generated database interactions like those found in frameworks like ActiveRecord or Hibernate.
In this example, I find and fix a similar issue in a Ruby on Rails application.
I noticed an unexpected database insertion alongside the expected SELECT statement. Instead of manually digging through the codebase, I used AppMap to scour the application for flaws and present me with comprehensive information about the exact locations of the issues within the codebase. Armed with this valuable insight, we are able to initiate fixes effectively.
If we open the Runtime Analysis tab in the AppMap plugin, we can see that it identified aINSERT into the analytics database, complete with IP addresses and other relevant information. A closer inspection reveals that this INSERT statement is occurring within a GET request. Oftentimes this is not the desired behavior and needs to be fixed. In our example, there are no side effects and the INSERT is simply logging traffic analytics data and may not need to be removed, however for purposes of our demonstration I will treat this as an issue that needs resolving.
- Please note that not every
INSERTin aGETrequest handler is a side effect and may be valid because it has no effect on the response; these types of side effects are not against spec. It’s perfectly fine to log API calls in your database or increment your API usage counter and still keep that API safe.
To pinpoint the source of this issue, click on the caller function link in the side panel. Doing so jumps to the call stack within the trace view, and we can see more information about the calling function in our side panel. By selecting the source path in the gray box, we are led to the code section we need to investigate. In our case, active_support/callbacks.rb
Active Support, a core dependency of Ruby on Rails, played a crucial role in initiating the action that causes the rogue INSERT to occur. While this is framework-dependent code and nothing we wrote, we don’t want to change it. However, the invoke before function does give us a clue as to what is happening. The function looks like this:
def invoke_before(arg)
@before.each { |b| b.call(arg) }
end
From this, we know that before any incoming request is processed, some function or action is getting called by one of our controllers to perform some action. Knowing that we need to find where the action gets created, by viewing which services the HTTP request “talks to” we can get a hint as to where the real issue might be
Tracing our steps further, we explore the HTTP request by switching to the dependency map, focusing on the GET /request and its connection to ActionPack, Active Support, and other parts of our code. Although most paths were internal or beyond our control, one of them led us to controllers/dashboard_controller.rb. We can navigate directly to it by clicking to expand the app/controllers box, then clicking on the DashboardController node and using the gray navigation box in the left panel.
Inside the DashboardController, I discover two important pieces of information:
1) That this controller is inheriting attributes from application_controller.rb
2) There is a skip_before_action method that skips actions that are invoked before the rest of the controller code executes
I would like to look at the parent controller to see what this controller is inheriting and what actions are being run. Upon examining the application controller, I observe the problematic before_actionmethod responsible for triggering a create analytic method which actually performs the work. This is the exact spot where the issue originates.
# frozen_string_literal: true
class DashboardController < ApplicationController
skip_before_action :has_info, :create_analytic
layout false, only: [:change_graph]
By adding this particular action to the skip_before_action list in our dasboard_controller and saving the changes, I am able to eliminate the unwanted INSERT behavior. It's worth noting again that in some cases, such as when logging user activity, leaving the INSERT intact might be preferable as it doesn’t cause any side effects.
This entire process of identifying and fixing the issue was made possible by utilizing AppMap to find and navigate our application’s runtime behavior, quickly guiding us to the resolution. Doing this without AppMap would have required a lot more time for investigation with multiple tools. But only AppMap can detect this kind of issue in the first place without us even knowing that there is an issue.
Links
⬇️ Download AppMap for VSCode and JetBrains: https://appmap.io/download
⭐ Star AppMap on GitHub: https://github.com/getappmap
🐦 Follow on Twitter: https://twitter.com/getappmap
💬 Join AppMap Slack: https://appmap.io/slack
ℹ️ Read the AppMap docs: https://appmap.io/docs
Top comments (0)