Fixes #3464

[kaitozaw]

#3464

Pull Request

Category

Bug

Feature/Issue Description

Q: Please give a brief summary of your feature/fix
A: This PR fixes order-dependent RSpec failures caused by ActiveRecord connections being disconnected in some specs, which left later database-dependent specs running against an empty or missing database.

Q: Give a technical rundown of what you have changed (if applicable)
A:
RSpec tests are executed in random order, but some specs were implicitly order-dependent.
Depending on the random seed, the number and type of failures varied.

The root cause was that a few specs explicitly call disconnect_all_active_record! (mainly for fork safety when starting servers).
Because the test suite uses SQLite in-memory databases, disconnecting ActiveRecord destroys the entire database.
If any subsequent specs required database access, they failed with missing table errors.

To fix this:

• Added a lightweight ActiveRecord snapshot/restore helper to spec_helper.rb
• Updated the affected specs to:
  • Take a snapshot of the ActiveRecord connection before disconnecting in before(:all)
  • Restore the connection (including schema/migrations) in after(:all)
• This ensures that specs which disconnect ActiveRecord clean up after themselves and do not affect later tests

As a result, the test suite is now order-independent and produces consistent results across different RSpec seeds.

[zinduolis]

Is there a specific reason for the double snapshot pattern in spec/beef/core/main/handlers/browser_details_handler_spec.rb and spec/beef/extensions/websocket_hooked_browser_spec.rb?

I noticed these two files behave differently from the others in this PR:

• Snapshot ... Restore: autorun_engine_spec.rb, redirector_spec.rb, and requester_spec.rb take a snapshot at the start and restore! at the end.
• Snapshot ... Snapshot: browser_details_handler_spec.rb and websocket_hooked_browser_spec.rb take a snapshot at the start and another snapshot at the end.

Was this intentional to preserve the new state?

[zinduolis]

Also, I've noticed one more potential improvement opportunity. Please explore if this helps.

Problem: The start_beef_server method establishes a database connection to run migrations (lines 249-262) but fails to disconnect it before forking (line 276). Inheriting an open SQLite connection in a child process is unsafe and can lead to database corruption or crashes.

Solution: Add disconnect_all_active_record! immediately before the fork block to ensure the child process starts with a clean slate.

    # ... migrations and setup ...

    # Generate a token for the server to respond with
    BeEF::Core::Crypto::api_token

    disconnect_all_active_record! # <--- ADD THIS

    # Initiate server start-up
    BeEF::API::Registrar.instance.fire(BeEF::API::Server, 'pre_http_start', http_hook_server)
    pid = fork do
      http_hook_server.start
    end

[kaitozaw]

Is there a specific reason for the double snapshot pattern in spec/beef/core/main/handlers/browser_details_handler_spec.rb and spec/beef/extensions/websocket_hooked_browser_spec.rb?

No, those were my mistakes. Thank you for pointing them out.

I’ve corrected both
spec/beef/core/main/handlers/browser_details_handler_spec.rb and
spec/beef/extensions/websocket_hooked_browser_spec.rb so that the ActiveRecord state is properly restored at the end of each spec. These specs are not executed by bundle exec rake --all, so I didn’t notice this issue earlier.

I’ve also added the improvement to spec_helper.rb that you recommended.

[zinduolis]

Verified Fixes

• spec/spec_helper.rb: The fork-safety issue is resolved. disconnect_all_active_record! is now correctly called at line 274, immediately before the fork block.
• spec/beef/core/main/handlers/browser_details_handler_spec.rb: The teardown logic at line 109 correctly uses SpecActiveRecordConnection.restore!(@__ar_config_snapshot).
• spec/beef/extensions/websocket_hooked_browser_spec.rb: The teardown logic at line 108 correctly uses SpecActiveRecordConnection.restore!(@__ar_config_snapshot).