Rack path traversal: exclude more query parameters
🔭 Context
Similar to Rack path traversal: exclude the note and body ... (!155279 - merged).
We are slowing rolling out the Rack middleware for path traversal checks. See https://gitlab.com/groups/gitlab-org/-/epics/13437+.
The middleware is only logging attempts. It is behind a feature flag.
During the roll out, we noticed that we could have an issue with the API requests that create commits. That API has a commit_message
and content
parameter that could contain strings that would be detected as path traversals even though they are valid requests.
We will need to excluded these from the Rack path traversal checker. That's what this MR does. The related issue is https://gitlab.com/gitlab-org/gitlab/-/issues/466563+.
⚙ What does this MR do and why?
- Exclude the
commit_message
andcontent
query parameters in the check path traversal middleware.
Note that specs were not updated because the excluded query parameter names are in the constant and we have a spec that will loop on that constant to assert the behavior.
The entire middleware is behind a feature flag. At the time of this writing, we're still rolling it out.
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
🦄 Screenshots or screen recordings
⚗ How to set up and validate locally
- Enable the middleware:
Feature.enable(:check_path_traversal_middleware)
- Have a project ready with the git repository initialized.
- Have a PAT ready.
1️⃣ On master
$ curl -vvv --request POST --header 'PRIVATE-TOKEN: <pat>' "http://gdk.test:8000/api/v4/projects/<project_id>/repository/files/test2%2Ftest.txt?branch=main&commit_message=foo%2F..%2Fbar&content=this+is+a+test"
$ curl -vvv --request POST --header 'PRIVATE-TOKEN: <pat>' "http://gdk.test:8000/api/v4/projects/<project_id>/repository/files/test3%2Ftest.txt?branch=main&commit_message=test&content=foo%2F..%2Fbar"
Look at log/application_json.log
:
{"severity":"WARN","time":"2024-06-11T13:47:14.751Z","correlation_id":"01J03RV1RMJRZ6129Q8D20BFGD","meta.caller_id":"POST /api/:version/projects/:id/repository/files/:file_path","meta.remote_ip":"172.16.123.1","meta.feature_category":"source_code_management","meta.user":"root","meta.user_id":1,"meta.project":"root/create-commits-api","meta.root_namespace":"root","meta.client_id":"user/1","method":"POST","fullpath":"/api/v4/projects/331/repository/files/test2%2Ftest.txt?branch=main\u0026commit_message=foo%2F..%2Fbar\u0026content=this+is+a+test","message":"Potential path traversal attempt detected","status":201,"class_name":"Gitlab::Middleware::PathTraversalCheck"}
{"severity":"WARN","time":"2024-06-11T13:47:50.157Z","correlation_id":"01J03RW4FNW9E7GKBCWCC49Q7A","meta.caller_id":"POST /api/:version/projects/:id/repository/files/:file_path","meta.remote_ip":"172.16.123.1","meta.feature_category":"source_code_management","meta.user":"root","meta.user_id":1,"meta.project":"root/create-commits-api","meta.root_namespace":"root","meta.client_id":"user/1","method":"POST","fullpath":"/api/v4/projects/331/repository/files/test3%2Ftest.txt?branch=main\u0026commit_message=test\u0026content=foo%2F..%2Fbar","message":"Potential path traversal attempt detected","status":201,"class_name":"Gitlab::Middleware::PathTraversalCheck"}
Two attempts have been caught.
2️⃣ With this MR
(Restart gdk when switching git branches. Middleware classes are loaded at boot time and are not updated even if the underlying ruby file has changed.)
$ curl -vvv --request POST --header 'PRIVATE-TOKEN: <pat>' "http://gdk.test:8000/api/v4/projects/<project_id>/repository/files/test4%2Ftest.txt?branch=main&commit_message=foo%2F..%2Fbar&content=this+is+a+test"
$ curl -vvv --request POST --header 'PRIVATE-TOKEN: <pat>' "http://gdk.test:8000/api/v4/projects/<project_id>/repository/files/test5%2Ftest.txt?branch=main&commit_message=test&content=foo%2F..%2Fbar"
Look at log/application_json.log
:
(empty)
The middleware excluded these query parameters properly