Skip to content
Snippets Groups Projects

Silent Mode: Block many outbound HTTP requests

Merged Michael Kozono requested to merge mk/silent-mode-block-outbound-gitlab-http into master
All threads resolved!

What does this MR do and why?

Silent Mode: Block many outbound HTTP requests.

Resolves #410048 (closed)

Related to #409662 (closed), #393639 (closed)

Screenshots or screen recordings

Silent Mode is disabled by default. Gitlab::HTTP behaves as usual:

➜  gitlab git:(mk/silent-mode-block-outbound-gitlab-http) bin/rails c
--------------------------------------------------------------------------------
 Ruby:         ruby 3.0.5p211 (2022-11-24 revision ba5cf0f7c5) [arm64-darwin21]
 GitLab:       16.0.0-pre (89f7dec48d7) EE
 GitLab Shell: 14.19.0
 PostgreSQL:   12.13
 Geo enabled:  yes
 Geo server:   primary
--------------------------------------------------------------------------------
Loading development environment (Rails 6.1.7.2)
[2] pry(main)> Gitlab::HTTP.post('https://httpstat.us/200', body: 'foo')
=> "200 OK"
[3] pry(main)> Gitlab::HTTP.post('https://httpstat.us/404', body: 'foo')
=> "404 Not Found"
[4] pry(main)> Gitlab::CurrentSettings.silent_mode_enabled?
=> false

Enable Silent Mode

[5] pry(main)> Gitlab::CurrentSettings.update!(silent_mode_enabled: true)
=> true
[6] pry(main)> Gitlab::CurrentSettings.silent_mode_enabled?
=> true

Gitlab::HTTP POST requests are blocked with an error that looks similar to an HTTParty error.

[7] pry(main)> Gitlab::HTTP.post('https://httpstat.us/200', body: 'foo')
Gitlab::HTTP::SilentModeBlockedError: only get, head, options, and trace methods are allowed in silent mode
from /Users/mkozonogitlab/Developer/gdk/gitlab/lib/gitlab/http.rb:99:in `raise_if_blocked_by_silent_mode'
[8] pry(main)> Gitlab::HTTP.post('https://httpstat.us/404', body: 'foo')
Gitlab::HTTP::SilentModeBlockedError: only get, head, options, and trace methods are allowed in silent mode
from /Users/mkozonogitlab/Developer/gdk/gitlab/lib/gitlab/http.rb:99:in `raise_if_blocked_by_silent_mode'

Logs:

{"severity":"INFO","time":"2023-05-11T02:18:11.928Z","correlation_id":null,"message":"Outbound HTTP request blocked","http_method":"Net::HTTP::Post","silent_mode_enabled":true}
{"severity":"INFO","time":"2023-05-11T02:18:15.683Z","correlation_id":null,"message":"Outbound HTTP request blocked","http_method":"Net::HTTP::Post","silent_mode_enabled":true}

Disable Silent Mode

[9] pry(main)> Gitlab::CurrentSettings.update!(silent_mode_enabled: false)
=> true
[10] pry(main)> Gitlab::CurrentSettings.silent_mode_enabled?
=> false

Gitlab::HTTP POST requests work again:

[11] pry(main)> Gitlab::HTTP.post('https://httpstat.us/200', body: 'foo')
=> "200 OK"
[12] pry(main)> Gitlab::HTTP.post('https://httpstat.us/404', body: 'foo')
=> "404 Not Found"

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Project webhooks will perform outbound POST requests using Gitlab::HTTP. So Silent Mode should block those requests.

How to set up a Project webhook

  1. Create a project if you have none
  2. In the Project main page, visit Settings > Webhooks
  3. Fill in URL with https://httpstat.us/200. This is just a URL which will always return 200 OK.
  4. Check the checkbox for Comments. Or whatever triggers you want to test.
  5. Click Add webhook

Confirm that the webhook succeeds when Silent Mode is disabled

  1. In the Project Hooks box, click the Test dropdown.
  2. Choose Comments to fire that trigger.
  3. Notice that with Silent Mode disabled, you will see a blue flash message Hook executed successfully: HTTP 200.

Enable Silent Mode

  1. Follow https://docs.gitlab.com/ee/administration/silent_mode/#enable-silent-mode.

Confirm that the webhook is blocked when Silent Mode is enabled

  1. In the Project Hooks box which was setup previously, click the Test dropdown.
  2. Choose Comments to fire that trigger.
  3. Notice the 500 error page indicating an error was raised prior to sending the HTTP request. (Non-development environments would respond with a less verbose 500.)

image

The GitLab Rails app should raise an error any time a POST, PUT, PATCH, or DELETE request is made from Gitlab::HTTP in the GitLab Rails app.

Webhooks are normally fired asynchronously. You can also observe a more comprehensive test:

Confirm that the webhook is blocked in Sidekiq jobs

  1. Create an issue in the project with the new webhook.

  2. gdk tail rails-background-jobs

  3. Add a comment to the issue.

  4. Notice a failed WebHookWorker job when Silent Mode is enabled:

    2023-05-11_03:03:18.38612 rails-background-jobs : {"severity":"WARN","time":"2023-05-11T03:03:18.385Z","retry":4,"queue":"default","backtrace":true,"version":0,"dead":false,"args":["1","[FILTERED]","note_hooks","{}"],"class":"WebHookWorker","jid":"ea437a5f0908b6b12d944ca4","created_at":"2023-05-11T03:03:18.143Z","correlation_id":"01H04C6KWM0R2PQB3MJDGCFBCB","meta.caller_id":"NewNoteWorker","meta.remote_ip":"172.16.123.1","meta.feature_category":"integrations","meta.user":"root","meta.user_id":1,"meta.project":"flightjs/Flight","meta.root_namespace":"flightjs","meta.client_id":"user/1","meta.root_caller_id":"Projects::NotesController#create","meta.related_class":"ProjectHook","meta.subscription_plan":"default","worker_data_consistency":"delayed","wal_locations":{},"wal_location_source":"primary","idempotency_key":"resque:gitlab:duplicate:default:07f3d1c6a9468f469900f9624467da6d2cf64448d61777a20e1fbe8e6fb8d289","size_limiter":"validated","enqueued_at":"2023-05-11T03:03:18.147Z","job_size_bytes":2410,"pid":57960,"message":"WebHookWorker JID-ea437a5f0908b6b12d944ca4: fail: 0.030162 sec","job_status":"fail","scheduling_latency_s":0.001032,"redis_calls":5,"redis_duration_s":0.000696,"redis_read_bytes":17,"redis_write_bytes":275,"redis_queues_calls":1,"redis_queues_duration_s":0.000202,"redis_queues_read_bytes":1,"redis_queues_write_bytes":123,"redis_shared_state_calls":4,"redis_shared_state_duration_s":0.000494,"redis_shared_state_read_bytes":16,"redis_shared_state_write_bytes":152,"db_count":1,"db_write_count":0,"db_cached_count":0,"db_replica_count":0,"db_primary_count":1,"db_main_count":1,"db_ci_count":0,"db_geo_count":0,"db_main_replica_count":0,"db_ci_replica_count":0,"db_replica_cached_count":0,"db_primary_cached_count":0,"db_main_cached_count":0,"db_ci_cached_count":0,"db_geo_cached_count":0,"db_main_replica_cached_count":0,"db_ci_replica_cached_count":0,"db_replica_wal_count":0,"db_primary_wal_count":0,"db_main_wal_count":0,"db_ci_wal_count":0,"db_geo_wal_count":0,"db_main_replica_wal_count":0,"db_ci_replica_wal_count":0,"db_replica_wal_cached_count":0,"db_primary_wal_cached_count":0,"db_main_wal_cached_count":0,"db_ci_wal_cached_count":0,"db_geo_wal_cached_count":0,"db_main_replica_wal_cached_count":0,"db_ci_replica_wal_cached_count":0,"db_replica_duration_s":0.0,"db_primary_duration_s":0.002,"db_main_duration_s":0.002,"db_ci_duration_s":0.0,"db_geo_duration_s":0.0,"db_main_replica_duration_s":0.0,"db_ci_replica_duration_s":0.0,"cpu_s":0.004533,"worker_id":"sidekiq_0","rate_limiting_gates":[],"duration_s":0.030162,"completed_at":"2023-05-11T03:03:18.178Z","load_balancing_strategy":"primary_no_wal","exception.class":"Gitlab::HTTP::SilentModeBlockedError","exception.message":"only get, head, options, and trace methods are allowed in silent mode","exception.backtrace":["lib/gitlab/http.rb:99:in `raise_if_blocked_by_silent_mode'","lib/gitlab/http.rb:48:in `perform_request'","app/services/web_hook_service.rb:123:in `make_request'","app/services/web_hook_service.rb:72:in `execute'","app/workers/web_hook_worker.rb:27:in `perform'","lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb:16:in `perform'","lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb:44:in `perform'","lib/gitlab/sidekiq_middleware/duplicate_jobs/server.rb:8:in `call'","lib/gitlab/sidekiq_middleware/worker_context.rb:9:in `wrap_in_optional_context'","lib/gitlab/sidekiq_middleware/worker_context/server.rb:19:in `block in call'","lib/gitlab/application_context.rb:118:in `block in use'","lib/gitlab/application_context.rb:118:in `use'","lib/gitlab/application_context.rb:57:in `with_context'","lib/gitlab/sidekiq_middleware/worker_context/server.rb:17:in `call'","lib/gitlab/sidekiq_status/server_middleware.rb:7:in `call'","lib/gitlab/sidekiq_versioning/middleware.rb:9:in `call'","lib/gitlab/sidekiq_middleware/query_analyzer.rb:7:in `block in call'","lib/gitlab/database/query_analyzer.rb:37:in `within'","lib/gitlab/sidekiq_middleware/query_analyzer.rb:7:in `call'","lib/gitlab/sidekiq_middleware/admin_mode/server.rb:14:in `call'","lib/gitlab/sidekiq_middleware/instrumentation_logger.rb:9:in `call'","lib/gitlab/sidekiq_middleware/batch_loader.rb:7:in `call'","lib/gitlab/sidekiq_middleware/extra_done_log_metadata.rb:7:in `call'","lib/gitlab/sidekiq_middleware/request_store_middleware.rb:10:in `block in call'","lib/gitlab/with_request_store.rb:17:in `enabling_request_store'","lib/gitlab/with_request_store.rb:10:in `with_request_store'","lib/gitlab/sidekiq_middleware/request_store_middleware.rb:9:in `call'","lib/gitlab/sidekiq_middleware/server_metrics.rb:83:in `block in call'","lib/gitlab/sidekiq_middleware/server_metrics.rb:110:in `block in instrument'","lib/gitlab/metrics/background_transaction.rb:33:in `run'","lib/gitlab/sidekiq_middleware/server_metrics.rb:110:in `instrument'","lib/gitlab/sidekiq_middleware/server_metrics.rb:82:in `call'","lib/gitlab/sidekiq_middleware/monitor.rb:10:in `block in call'","lib/gitlab/sidekiq_daemon/monitor.rb:46:in `within_job'","lib/gitlab/sidekiq_middleware/monitor.rb:9:in `call'","lib/gitlab/sidekiq_middleware/size_limiter/server.rb:13:in `call'","lib/gitlab/sidekiq_logging/structured_logger.rb:21:in `call'"],"db_duration_s":0.000364}

Why are these errors ok?

This isn't ideal UX, but this MR is intended as a starting point for iteration. The Silent Mode feature is currently an "Experiment". The UX impact and potential mitigation of an error at each call site must be evaluated on a case-by-case basis. Importantly, end users won't interact with GitLab in Silent Mode. Only sysadmins testing non-production instances will perform tests with Silent Mode enabled.

There is no feature flag, since the Application Setting silent_mode_enabled (with no UI control) acts as one. It would be redundant to cover this toggle with another.

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 Michael Kozono

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Pedro Pombeiro
  • Pedro Pombeiro
  • Michael Kozono added 1 commit

    added 1 commit

    • 950c1aa9 - Single quotes for file consistency

    Compare with previous version

  • Michael Kozono added 1 commit

    added 1 commit

    • 42484477 - Dry up silent mode unit tests

    Compare with previous version

  • Michael Kozono added 1 commit

    added 1 commit

    • eed26314 - Dry up silent mode unit tests

    Compare with previous version

  • Author Maintainer

    @ameyadarshan Sorry, I force pushed over one of my new commits in which I locally applied some suggestions, and it removed your required approval.

  • Pedro Pombeiro
  • Pedro Pombeiro requested review from @alipniagov

    requested review from @alipniagov

  • Pedro Pombeiro requested review from @mkaeppler and removed review request for @alipniagov

    requested review from @mkaeppler and removed review request for @alipniagov

  • Pedro Pombeiro requested review from @stanhu and removed review request for @pedropombeiro, @mkaeppler, and @ameyadarshan

    requested review from @stanhu and removed review request for @pedropombeiro, @mkaeppler, and @ameyadarshan

  • Pedro Pombeiro approved this merge request

    approved this merge request

  • Pedro Pombeiro requested review from @mkaeppler and @ameyadarshan and removed review request for @stanhu

    requested review from @mkaeppler and @ameyadarshan and removed review request for @stanhu

  • Matthias Käppler
  • Matthias Käppler
  • Michael Kozono added 1207 commits

    added 1207 commits

    Compare with previous version

  • Michael Kozono added 1 commit

    added 1 commit

    Compare with previous version

  • Matthias Käppler approved this merge request

    approved this merge request

  • Ameya Darshan approved this merge request

    approved this merge request

  • Matthias Käppler resolved all threads

    resolved all threads

  • Matthias Käppler enabled an automatic merge when the pipeline for 636fa3b2 succeeds

    enabled an automatic merge when the pipeline for 636fa3b2 succeeds

  • mentioned in commit 4e94a044

  • added workflowstaging label and removed workflowcanary label

  • Michael Kozono mentioned in merge request !122139 (merged)

    mentioned in merge request !122139 (merged)

  • mentioned in issue #415227 (closed)

  • Bojan Marjanovic mentioned in merge request !125226 (merged)

    mentioned in merge request !125226 (merged)

  • mentioned in issue #419052 (closed)

  • Furkan Ayhan mentioned in merge request !125024 (merged)

    mentioned in merge request !125024 (merged)

  • Please register or sign in to reply
    Loading