Fix strict-CSP rendering, secure the evaluator, release 0.1.6#6
Merged
Conversation
CI has never passed on this repository: the test job ran "bin/rails db:test:prepare test", but the engine binstub has no test command, and the lint job ran rubocop-rails-omakase against a codebase that was formatted with Standard (e48507c), so their spacing rules disagreed on every literal. Make Standard the linter of record. README, the gemspec's development dependency, and the existing formatting all already point at Standard; omakase was only present because the engine generator scaffolded it. Remove rubocop-rails-omakase, .rubocop.yml, and the rubocop binstub, and run "bundle exec standardrb" in CI. Ignore the auto-generated dummy schema instead of reformatting it. Give the Rakefile real test tasks: "rake test" (as the README always claimed) for unit/integration tests and "rake test:system" for browser tests, with capybara + selenium-webdriver added for the latter. Commit an empty dummy-app schema so rails/test_help can maintain the test database, require minitest/mock for stubbing, and point the CI screenshot artifact at the dummy app's tmp/screenshots. Pin Ruby 3.4.4 in .ruby-version; CI's setup-ruby now reads it instead of duplicating the version, and the lockfile (which already carries the x86_64-linux platform for Linux CI/installs) resolves as-is.
The console captured the binding of ConsoleController#index, so input
was evaluated inside the engine's lexical namespace: ApplicationController
resolved to SlashConsole::ApplicationController, self was the controller
instance, and app-level constants sometimes needed a leading :: to
disambiguate - unlike bin/rails console.
Pass web-console an explicitly created top-level binding instead.
TOPLEVEL_BINDING.eval("binding") yields a binding whose self is main and
whose constant lookup starts at Object, matching bin/rails console, while
still seeing everything Rails has loaded. Because each console session
gets a fresh binding, local variables persist across evaluations within
a session (a REPL essential) but cannot leak between sessions or
pollute TOPLEVEL_BINDING for the rest of the process.
In production, Basic authentication lived in ConsoleController, which only guards the console page. web-console evaluates code through PUT/POST <mount_point>/repl_sessions/:id, and those requests are served directly by WebConsole::Middleware - they never reach the router, so no controller filter can see them. A session ID (visible to anyone who loaded the page, or brute-forceable in principle) therefore acted as an unauthenticated bearer token for arbitrary code execution until the process restarted. Move authentication into SlashConsole::BasicAuthMiddleware, inserted immediately before WebConsole::Middleware. It guards the console page paths at the standard /rails mount and everything under WebConsole::Middleware.mount_point, using the same ADMIN_USERNAME / ADMIN_PASSWORD environment variables (503 when unset, 401 on missing or wrong credentials). The path pattern deliberately excludes other /rails/* paths: unmatched requests cascade past the engine to the host app, which owns e.g. /rails/active_storage/*, and those must keep working. The controller filters remain as a second layer for apps that mount the engine at a custom path, now delegating to the middleware's credential checks so there is one definition of "authorized". Also stop inserting a second WebConsole::Middleware: web-console's own railtie already inserts one before ActionDispatch::DebugExceptions, so the engine's insert_after produced a duplicate pair sandwiching DebugExceptions. The engine now only inserts the auth middleware, anchored after web_console.insert_middleware so the target exists. A middleware stack test pins all of this: exactly one WebConsole::Middleware, auth directly in front of it, and the protected path pattern covering every path the engine serves while leaving cascaded host-app paths alone.
The dummy app's strict nonce CSP did not actually reproduce the blank console that 0.1.5 shipped to production. Rails places ContentSecurityPolicy::Middleware below WebConsole::Middleware, so on the way out the CSP middleware generates and memoizes the request nonce before web-console injects its scripts, and injection picks up a valid nonce even without our view ever asking for one. Production apps (Photogram Golden among them) hoist the CSP middleware above ShowExceptions so error responses carry the policy too. That also hoists it above WebConsole::Middleware, inverting the order: web-console injects first, finds no memoized nonce, and stamps nonce="" on every script, after which the CSP middleware mints a fresh nonce for the header. Nonempty header nonce, empty script nonces, blocked scripts, blank console - the 0.1.5 failure. The view's early content_security_policy_nonce call is what makes both consumers agree, because the first caller memoizes the nonce into the Rack env. Apply the same move_before to the dummy app so the suite covers the hostile arrangement, and replace the placeholder navigation test with dedicated coverage: the header nonce is nonempty and appears on the page stylesheet and on every injected script with no nonce="" left behind, and the policy stays free of unsafe-inline/unsafe-eval. A headless-Chrome system test proves the result where it matters - the >> prompt mounts, the stylesheet actually applies (body margin is zeroed), and 6 * 7 evaluates to 42. Each of these fails against the 0.1.5 page markup.
Document the strict-CSP rendering fix, the evaluator authentication gap, the top-level evaluation binding, and the duplicate middleware removal in the changelog, and refresh the README's description of how production authentication and the test suite work.
raghubetina
changed the title
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
1 participant
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.
Prepares the 0.1.6 patch release that makes SlashConsole safe and usable as the permanent fix for production consoles in strict-CSP apps (Photogram Golden is currently pinned to commit f82d8ab awaiting this release).
Rendering under strict CSP
move_before ActionDispatch::ShowExceptions, ...ContentSecurityPolicy::Middleware, as in Photogram Golden). With the CSP middleware above web-console, injection happens before the header nonce is minted — this is exactly how 0.1.5 produced a nonempty header nonce withnonce=""scripts and a blank console. Withoutmove_before, the bug is invisible in tests.nonce=""remains, and the policy contains nounsafe-inline/unsafe-eval. A headless-Chrome system test proves the>>prompt mounts, the stylesheet applies, and6 * 7evaluates to42. All of these fail against the 0.1.5 markup.Security: evaluator endpoints were unauthenticated
web-console serves
PUT/POST <mount_point>/repl_sessions/:iddirectly fromWebConsole::Middleware, so the controller's Basic auth never saw them — a session ID was an unauthenticated bearer token for code execution until process restart. Authentication now lives inSlashConsole::BasicAuthMiddleware, inserted directly beforeWebConsole::Middleware, covering the console page and everything under the evaluator mount point with the sameADMIN_USERNAME/ADMIN_PASSWORD(503 when unset). The path pattern deliberately leaves other/rails/*paths (e.g. ActiveStorage) alone, and the controller filters remain as a second layer for custom mounts. Also removes the duplicateWebConsole::Middlewarethe engine inserted alongside web-console's own.Console evaluates at the top level
Input used to evaluate in
ConsoleController#index's binding, soApplicationControllerresolved toSlashConsole::ApplicationControllerand app constants could need a leading::. Each session now gets a freshTOPLEVEL_BINDING.eval("binding"):selfismain, constants resolve likebin/rails console, locals persist within a session but not across sessions.CI repair
CI had never passed: the test job ran a nonexistent
bin/rails testcommand and the lint job ran omakase RuboCop against Standard-formatted code. Standard is now the linter of record (README/gemspec already said so);rake testandrake test:systemexist and CI runs both, plus Chrome for the system test. Ruby is pinned via.ruby-versionand the lockfile supportsx86_64-linux.