Deep-review hardening: auth in all deployed envs, dependency floors, session cap#7
Merged
Merged
Conversation
The engine force-enables web-console in every environment and clears its IP allowlist, but both the auth middleware and the controller filters only engaged when Rails.env.production?. A deployment running any other environment name - staging, preview, demo - therefore served a completely unauthenticated console to the internet. Invert the gate: development and test are the only open environments, and anything else requires ADMIN_USERNAME/ADMIN_PASSWORD (or gets the 503 setup message until they are set), so unrecognized environments now fail safe. The environment predicate lives on BasicAuthMiddleware and the controller's second-layer filters share it. Also stop splatting Rack's decoded Basic credentials into authorized?. Rack 3 rejects credential arrays that aren't exactly two elements in Request#basic?, but Rack 2 (still possible under Rails 7.0, which the gemspec supports) does not, so a header whose decoded value lacks a colon would raise ArgumentError and 500 instead of returning 401. Destructuring pads with nil and authorized? already normalizes with to_s, so malformed headers now fail closed. While here, delete the engine's unused application layout: every render uses layout false, and it referenced a stylesheet that does not ship with the gem.
WebConsole::Session.inmemory_storage is an append-only hash: every console page load stores a session holding a live binding, and nothing ever removes it, so repeated loads pin memory until the process restarts. Drop the oldest entries once 50 accumulate (Hash preserves insertion order, so shift evicts oldest-first). A tab whose session was evicted gets web-console's normal "session is no longer available" message and recovers on reload. Also remove the CSRF skip_before_action: the console page is GET-only and Rails does not verify authenticity tokens on GET, so the skip (and its defined? guard) did nothing.
Require web-console >= 4.2.1. The gemspec accepted >= 4.0, but CSP nonces on injected assets only arrived in 4.1.0 (web-console #296) and Rack 3 / Rails 7.1 support in 4.2.1 - an app resolving anything older gets the silent blank-console-under-CSP failure that 0.1.6 fixed. Enable rubygems_mfa_required so pushes and yanks of this gem require a multi-factor-authenticated RubyGems session; a gem that ships a production console is a worthwhile supply-chain target. Bump actions/checkout and actions/upload-artifact to their current majors (v6), clearing the Node 20 deprecation warnings on every run. This supersedes the open dependabot PRs, which predate CI working at all.
Changelog for the deep-review hardening pass, plus README updates: the authentication section now describes all deployed environments rather than just production, and notes the brute-force caveat on the password prompt, the multi-process session limitation, and that the gem overrides the host app's web_console settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Findings from a deep review of the gem after the 0.1.6 release, prepared as 0.1.7.
Security
production— astaging/previewdeployment served an unauthenticated console (verified empirically). Development and test are now the only open environments; everything else fails safe (503 until credentials are configured, then Basic auth). Tests cover staging anonymous/valid on both the page and the evaluator.rubygems_mfa_requiredenabled in the gemspec.Fixes / hygiene
actions/checkoutandactions/upload-artifactto v6, clearing Node 20 deprecation warnings and superseding dependabot PRs Bump actions/checkout from 4 to 6 #3/Bump actions/upload-artifact from 4 to 6 #4.