Skip to content

Log the status code in the path traversal check middleware

David Fernandez requested to merge 413766-add-status-code-to-logs into master

🌳 Context

In Rack middleware for path traversal checks (!123477 - merged), we introduced a rack middleware with this idea of checking the request path, if a path traversal is detected, reject the request. See #413766+ for more details.

Since middlewares are executed for all requests, we are not starting with the "reject the request" part (yet). Instead, we simply log the path traversal attempt and additionally we measure the execution time of that check. This is the simulation phase.

For an additional safety net, the middleware is behind a feature flag. See the rollout issue: https://gitlab.com/gitlab-org/gitlab/-/issues/415460.

🔍 What does this MR do and why?

This brings a small update:

  • if a path traversal is detected, we log it. This MR will add the status code that comes from executing @app.call.

This helps with debugging and analysing the logs. We basically need a way to detect false positives: requests that have been detected as path traversal attempts but are valid/genuine requests to GitLab.

One way to do so is to check requests that are catched by the middleware and have a successful status code, such as 200.

🖼 Screenshots or screen recordings

No UI changes 🦄.

⚙ How to set up and validate locally

  1. In a rails console, enable the middleware and enable the execution time logging:
Feature.enable(:check_path_traversal_middleware)
  1. Start the server.
  2. Access http://gdk.test:8000/foo%2F..%2Fbar
  3. Check log/application_json.log

You will see these logs:

{"severity":"WARN","time":"2023-11-30T16:49:04.586Z","correlation_id":"01HGGJ4EZTZV5FDX4RYHZ5M1PJ","meta.caller_id":"ApplicationController#route_not_found","meta.remote_ip":"172.16.123.1","meta.user":"root","meta.user_id":1,"meta.client_id":"user/1","method":"GET","fullpath":"/foo%2F..%2Fbar","message":"Potential path traversal attempt detected","status":404,"class_name":"Gitlab::Middleware::PathTraversalCheck"}

The http status code is properly logged ✅

🏎 MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by David Fernandez

Merge request reports