feat(frontend): comprehensive UI audit — accessibility, Catppuccin theming, Railsboot removal#706
Merged
Conversation
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…pendencies Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…behavior Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ity guidelines Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… maintainability Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… and layout Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ayout Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…and error handling Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…nd accessibility Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… accessibility Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…k config Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…tstrap Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…layout Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ross views Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ility Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/system/agents/tabs_controller_regression_spec.rb (1)
53-59: 🧹 Nitpick | 🔵 TrivialHarden this regression check and align the example name.
Use token inclusion instead of full-string equality (order/spacing-safe), and rename the spec to mention both click and keydown actions.
Proposed test tweak
- it "uses click->tabs#switch data-action on tab links" do + it "uses click and keydown data-action handlers on tab links" do agent_page.visit_page(agent) tab_links = page.all("[data-tabs-target='tab']") tab_links.each do |tab_link| - expect(tab_link[:"data-action"]).to eq("click->tabs#switch keydown->tabs#keydown") + actions = tab_link[:"data-action"].to_s.split + expect(actions).to include("click->tabs#switch", "keydown->tabs#keydown") end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/system/agents/tabs_controller_regression_spec.rb` around lines 53 - 59, Rename the example to mention both click and keydown (e.g., "uses click and keydown actions on tab links") and change the assertion that checks each tab link's data-action: instead of expecting full-string equality for tab_links[i][:"data-action"], assert that the attribute includes the tokens "click->tabs#switch" and "keydown->tabs#keydown" (order/spacing agnostic) so the spec remains robust; keep using agent_page.visit_page(agent) and the tab_links = page.all("[data-tabs-target='tab']") collection to locate targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/support/system_helpers.rb`:
- Around line 62-63: The logout helper uses first("form[action*='sign_out']",
visible: :all) which can return nil and lead to NoMethodError when calling
logout_form.native; replace the call to first with
find("form[action*='sign_out']", visible: :all, wait: 5) so Capybara will wait
for the element (raising Capybara::ElementNotFound on timeout) and then call
page.execute_script("arguments[0].submit()", logout_form.native) as before;
update the logout_form assignment in spec/support/system_helpers.rb accordingly.
In `@spec/system/navigation/navbar_navigation_spec.rb`:
- Around line 37-41: The spec currently manipulates navbar internals directly;
extract those interactions into a page object (e.g., class NavbarPage) with
methods open_tools_dropdown, navigate_to_word_lists, open_user_menu, and
go_to_profile_edit; implement open_tools_dropdown to click the dropdown-toggle,
navigate_to_word_lists to find the word_lists link and perform the JS click
(encapsulating the page.execute_script usage), and
open_user_menu/go_to_profile_edit to encapsulate the user menu flow; then update
navbar_navigation_spec.rb to call these page object methods instead of using
first("a[href=...]") / page.execute_script directly so the spec reads
intentfully and hides implementation details.
- Around line 40-41: The tests use first("a[href='#{word_lists_path}']",
visible: :all).native which will raise NoMethodError when the selector doesn't
match; replace these with Capybara's find("a[href='#{word_lists_path}']",
visible: :all, wait: 5) (or another explicit wait) so Capybara raises
Capybara::ElementNotFound and provides better diagnostics, then pass the found
element to page.execute_script("arguments[0].click()", ... ) or call click on
it; make the same replacement for the other occurrence around lines 56-57 to
avoid nil.native usage.
---
Outside diff comments:
In `@spec/system/agents/tabs_controller_regression_spec.rb`:
- Around line 53-59: Rename the example to mention both click and keydown (e.g.,
"uses click and keydown actions on tab links") and change the assertion that
checks each tab link's data-action: instead of expecting full-string equality
for tab_links[i][:"data-action"], assert that the attribute includes the tokens
"click->tabs#switch" and "keydown->tabs#keydown" (order/spacing agnostic) so the
spec remains robust; keep using agent_page.visit_page(agent) and the tab_links =
page.all("[data-tabs-target='tab']") collection to locate targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6f22694-13c3-4a28-88f3-175681cc08dd
📒 Files selected for processing (4)
spec/components/agent_detail_tabs_component_spec.rbspec/support/system_helpers.rbspec/system/agents/tabs_controller_regression_spec.rbspec/system/navigation/navbar_navigation_spec.rb
Comment on lines
+62
to
63
| logout_form = first("form[action*='sign_out']", visible: :all) | ||
| page.execute_script("arguments[0].submit()", logout_form.native) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the `first(...).native` dereference pattern in system helpers
rg -n -C2 --type=ruby 'logout_form = first\(|logout_form\.native' spec/support/system_helpers.rbRepository: unclesp1d3r/CipherSwarm
Length of output: 443
🏁 Script executed:
#!/bin/bash
# Check for other `first()` calls in system_helpers.rb that might have the same issue
rg -n 'first\(' spec/support/system_helpers.rbRepository: unclesp1d3r/CipherSwarm
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Also check the full file to understand the test context and any error handling patterns
wc -l spec/support/system_helpers.rbRepository: unclesp1d3r/CipherSwarm
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Get the full context of the file to verify test patterns
cat -n spec/support/system_helpers.rb | head -100Repository: unclesp1d3r/CipherSwarm
Length of output: 4851
Replace first with find to prevent nil dereference crashes in async tests
Line 62 using first() returns nil when the logout form is not immediately available, causing NoMethodError on line 63 when calling .native. This creates flaky tests that fail unpredictably.
Replace with find(..., wait: 5) for deterministic behavior and proper Capybara waiting:
Diff
- logout_form = first("form[action*='sign_out']", visible: :all)
+ logout_form = find("form[action*='sign_out']", visible: :all, wait: 5)
page.execute_script("arguments[0].submit()", logout_form.native)find() raises Capybara::ElementNotFound with context on timeout instead of silently returning nil, and respects async rendering delays.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logout_form = first("form[action*='sign_out']", visible: :all) | |
| page.execute_script("arguments[0].submit()", logout_form.native) | |
| logout_form = find("form[action*='sign_out']", visible: :all, wait: 5) | |
| page.execute_script("arguments[0].submit()", logout_form.native) |
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 62-62: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: page.execute_script("arguments[0].submit()", logout_form.native)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/support/system_helpers.rb` around lines 62 - 63, The logout helper uses
first("form[action*='sign_out']", visible: :all) which can return nil and lead
to NoMethodError when calling logout_form.native; replace the call to first with
find("form[action*='sign_out']", visible: :all, wait: 5) so Capybara will wait
for the element (raising Capybara::ElementNotFound on timeout) and then call
page.execute_script("arguments[0].submit()", logout_form.native) as before;
update the logout_form assignment in spec/support/system_helpers.rb accordingly.
Comment on lines
+37
to
41
| click_button(class: "dropdown-toggle", text: /Tools/i) | ||
|
|
||
| # Use JavaScript to click the link to bypass Bootstrap dropdown timing issues | ||
| word_lists_link = find("a[href='#{word_lists_path}']", visible: :all) | ||
| word_lists_link = first("a[href='#{word_lists_path}']", visible: :all) | ||
| page.execute_script("arguments[0].click()", word_lists_link.native) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move navbar interaction flow into a page object
These changed flows still encode navbar internals directly in the spec. Extract them to a page object method (e.g., open_tools_dropdown, navigate_to_word_lists, open_user_menu, go_to_profile_edit) to keep system specs stable and intention-focused.
As per coding guidelines, spec/system/**/*.rb: System tests should use the Page Object Pattern organized under spec/support/page_objects/.
Also applies to: 53-57
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 40-40: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: page.execute_script("arguments[0].click()", word_lists_link.native)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/system/navigation/navbar_navigation_spec.rb` around lines 37 - 41, The
spec currently manipulates navbar internals directly; extract those interactions
into a page object (e.g., class NavbarPage) with methods open_tools_dropdown,
navigate_to_word_lists, open_user_menu, and go_to_profile_edit; implement
open_tools_dropdown to click the dropdown-toggle, navigate_to_word_lists to find
the word_lists link and perform the JS click (encapsulating the
page.execute_script usage), and open_user_menu/go_to_profile_edit to encapsulate
the user menu flow; then update navbar_navigation_spec.rb to call these page
object methods instead of using first("a[href=...]") / page.execute_script
directly so the spec reads intentfully and hides implementation details.
Comment on lines
+40
to
41
| word_lists_link = first("a[href='#{word_lists_path}']", visible: :all) | ||
| page.execute_script("arguments[0].click()", word_lists_link.native) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify `first(...).native` is used without a nil-safe guard in this spec
rg -n -C2 --type=ruby 'word_lists_link = first\(|edit_profile_link = first\(|\.native\)' spec/system/navigation/navbar_navigation_spec.rbRepository: unclesp1d3r/CipherSwarm
Length of output: 721
Use find() instead of first().native for clearer failure diagnostics
Lines 40-41 and 56-57 use first(...) followed by .native without nil-check. If the selector fails to match, .native raises NoMethodError on nil instead of the expected Capybara::ElementNotFound. Replace with find() and explicit wait time.
Proposed fix
- word_lists_link = first("a[href='#{word_lists_path}']", visible: :all)
+ word_lists_link = find("a[href='#{word_lists_path}']", visible: :all, wait: 5)
page.execute_script("arguments[0].click()", word_lists_link.native)- edit_profile_link = first("a[href='#{edit_user_registration_path}']", visible: :all)
+ edit_profile_link = find("a[href='#{edit_user_registration_path}']", visible: :all, wait: 5)
page.execute_script("arguments[0].click()", edit_profile_link.native)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| word_lists_link = first("a[href='#{word_lists_path}']", visible: :all) | |
| page.execute_script("arguments[0].click()", word_lists_link.native) | |
| word_lists_link = find("a[href='#{word_lists_path}']", visible: :all, wait: 5) | |
| page.execute_script("arguments[0].click()", word_lists_link.native) |
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 40-40: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: page.execute_script("arguments[0].click()", word_lists_link.native)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/system/navigation/navbar_navigation_spec.rb` around lines 40 - 41, The
tests use first("a[href='#{word_lists_path}']", visible: :all).native which will
raise NoMethodError when the selector doesn't match; replace these with
Capybara's find("a[href='#{word_lists_path}']", visible: :all, wait: 5) (or
another explicit wait) so Capybara raises Capybara::ElementNotFound and provides
better diagnostics, then pass the found element to
page.execute_script("arguments[0].click()", ... ) or call click on it; make the
same replacement for the other occurrence around lines 56-57 to avoid nil.native
usage.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the removal of the custom “Railsboot” view component layer and replaces it with direct ERB + Bootstrap markup, while also improving accessibility (ARIA, keyboard tabs, skip link) and introducing a Catppuccin-based dark theme plus self-hosted fonts.
Changes:
- Replace Railsboot-based headers/cards/tables/nav/blank-slate UI with plain Bootstrap HTML across views and components.
- Add accessibility improvements:
<button>-based dropdown toggles, skip link + offcanvas mobile nav, keyboard-navigable Stimulus tabs, ARIA labels/scope attributes. - Introduce Catppuccin Macchiato theme overrides and self-hosted fonts; update asset build steps to copy font files.
Reviewed changes
Copilot reviewed 212 out of 214 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/components/railsboot/heading_component_test.rb | Removes Railsboot heading component test (component removed). |
| tessl.json | Adds Tessl-managed tooling integrations. |
| spec/system/navigation/navbar_navigation_spec.rb | Updates navbar system specs for button-based dropdown toggles. |
| spec/system/agents/tabs_controller_regression_spec.rb | Updates expectation for new keyboard action on tabs. |
| spec/support/system_helpers.rb | Updates helpers to find button-based user dropdown + logout form selection. |
| spec/components/toast_notification_component_spec.rb | Adds coverage for danger toasts disabling autohide. |
| spec/components/agent_detail_tabs_component_spec.rb | Updates expected data-action to include keyboard handler. |
| package.json | Adds @fontsource dependencies and extends font copy build script. |
| GOTCHAS.md | Documents new frontend/accessibility gotchas and workflow notes. |
| DESIGN.md | Adds design context and accessibility requirements documentation. |
| app/views/tasks/show.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb; minor semantic tweaks. |
| app/views/tasks/logs.html.erb | Same header replacement + table header semantics. |
| app/views/system_health/index.html.erb | Replaces Railsboot cards/blank-slate with Bootstrap markup; text color tweaks. |
| app/views/shared/attack_resource/view_file.html.erb | Replaces Railsboot header/buttons with Bootstrap; layout adjustments. |
| app/views/shared/attack_resource/_show.html.erb | Replaces Railsboot header/actions with Bootstrap + ARIA labels. |
| app/views/shared/attack_resource/_new.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/shared/attack_resource/_index.html.erb | Replaces Railsboot table/actions with Bootstrap table + responsive wrapper. |
| app/views/shared/attack_resource/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links. |
| app/views/shared/attack_resource/_edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/projects/show.html.erb | Replaces Railsboot header/actions with Bootstrap; adds ARIA on action group. |
| app/views/projects/new.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/projects/edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/projects/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links. |
| app/views/partials/_sidebar.html.erb | Adds aria-label to main navigation list. |
| app/views/partials/_sidebar_navbar_items.html.erb | Adds offcanvas account/tools navigation list for mobile. |
| app/views/partials/_navbar.html.erb | Replaces Railsboot navbar with Bootstrap markup; uses <button> dropdown toggles. |
| app/views/layouts/application.html.erb | Adds viewport meta, skip link, responsive layout grid, mobile offcanvas, inline flash rendering, z-index utility. |
| app/views/hash_lists/show.html.erb | Replaces Railsboot header/nav with Bootstrap; improves table semantics and live region. |
| app/views/hash_lists/new.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/hash_lists/index.html.erb | Replaces Railsboot header with Bootstrap card; wraps table in responsive container. |
| app/views/hash_lists/edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/hash_lists/_hash_list.html.erb | Adds ARIA labels/roles to action button group. |
| app/views/hash_lists/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links. |
| app/views/errors/unsupported_version.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/unknown_error.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/service_unavailable.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/route_not_found.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/resource_not_found.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/not_authorized.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/not_acceptable.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/internal_server_error.html.erb | Updates muted text class to text-body-secondary. |
| app/views/errors/bad_request.html.erb | Updates muted text class to text-body-secondary. |
| app/views/devise/registrations/edit.html.erb | Replaces Railsboot header/errors with Bootstrap card/breadcrumb + inline alert. |
| app/views/campaigns/show.html.erb | Replaces Railsboot header/blank-slate/stepper/spinners with Bootstrap markup; adds live region wrapper. |
| app/views/campaigns/new.html.erb | Replaces Railsboot header/blank-slate with Bootstrap markup. |
| app/views/campaigns/index.html.erb | Replaces Railsboot header/blank-slate with Bootstrap markup; responsive table wrapper. |
| app/views/campaigns/edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/campaigns/_recent_cracks.html.erb | Replaces Railsboot blank-slate/table with Bootstrap markup; updates muted text styling. |
| app/views/campaigns/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links. |
| app/views/campaigns/_eta_summary.html.erb | Replaces Railsboot alert component with Bootstrap alert; updates muted text styling. |
| app/views/campaigns/_error_log.html.erb | Replaces Railsboot blank-slate/table with Bootstrap markup; updates muted text styling. |
| app/views/campaigns/_campaign.html.erb | Replaces Railsboot button group and adds responsive wrapper for nested attacks table. |
| app/views/campaigns/_attack_stepper_line.html.erb | Replaces Railsboot stepper item/button components with Bootstrap markup. |
| app/views/attacks/show.html.erb | Replaces Railsboot header/actions with Bootstrap card/breadcrumb. |
| app/views/attacks/new.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/attacks/edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/attacks/_mask_form.html.erb | Adjusts helper text styling away from text-muted. |
| app/views/attacks/_hybrid_form.html.erb | Adjusts helper text styling away from text-muted. |
| app/views/attacks/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links; adjusts helper text styling. |
| app/views/attacks/_attack.html.erb | Updates muted text class to text-body-secondary. |
| app/views/agents/show.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb; adds live region on agent container; adds ARIA on action group. |
| app/views/agents/new.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/agents/index.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/agents/edit.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/agents/_overview_tab.html.erb | Updates muted text class to text-body-secondary. |
| app/views/agents/_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links; updates muted text class. |
| app/views/agents/_errors_tab.html.erb | Adds scope="col" and replaces blank-slate with Bootstrap markup. |
| app/views/agents/_configuration_tab.html.erb | Updates muted text class to text-body-secondary. |
| app/views/agents/_cards.html.erb | Replaces Railsboot blank-slate with Bootstrap markup. |
| app/views/agents/_capabilities_tab.html.erb | Updates muted text + replaces blank-slate with Bootstrap markup; adds scope="col". |
| app/views/agents/_agent.html.erb | Adds ARIA labels/roles to action button group. |
| app/views/admin/new_user.html.erb | Replaces Railsboot header with Bootstrap card/breadcrumb. |
| app/views/admin/index.html.erb | Replaces Railsboot header with Bootstrap card; improves table semantics + responsive wrappers. |
| app/views/admin/_user.html.erb | Adds ARIA labels to lock/unlock buttons and button group. |
| app/views/admin/_user_form.html.erb | Replaces Railsboot errors/buttons with inline Bootstrap alert + links. |
| app/views/admin/_project.html.erb | Adds ARIA labels/roles to action button group. |
| app/javascript/controllers/tabs_controller.js | Adds keyboard navigation + roving tabindex and role initialization for tabs. |
| app/components/toast_notification_component.rb | Adds autohide? behavior based on variant. |
| app/components/toast_notification_component.html.erb | Binds autohide value to autohide?. |
| app/components/task_actions_component.html.erb | Updates muted text class to text-body-secondary. |
| app/components/system_health_card_component.html.erb | Replaces Railsboot card usage with Bootstrap card markup. |
| app/components/skeleton_loader_component.html.erb | Replaces Railsboot card usage and removes inline styles via new utility classes. |
| app/components/railsboot/** (many files) | Removes Railsboot components (toast/table/stepper/spinner/etc.) now unused. |
| app/components/navbar_dropdown_component.html.erb | Changes dropdown trigger from <a href=\"#\"> to <button>. |
| app/components/error_modal_component.rb | Updates comment to reflect Bootstrap modal layout. |
| app/components/error_modal_component.html.erb | Replaces Railsboot modal with direct Bootstrap modal markup. |
| app/components/campaign_progress_component.html.erb | Replaces inline flex styles with Bootstrap grid classes; updates muted text class. |
| app/components/agent_status_card_skeleton_component.html.erb | Replaces Railsboot card with Bootstrap markup. |
| app/components/agent_status_card_component.html.erb | Replaces Railsboot card/button group with Bootstrap markup. |
| app/components/agent_detail_tabs_component.html.erb | Replaces Railsboot nav tabs with explicit Bootstrap/ARIA tab markup + keyboard action. |
| app/assets/stylesheets/components/_agent_tabs.scss | Adds reduced-motion handling and updates background variable for dark theme. |
| app/assets/stylesheets/application.bootstrap.scss | Imports Catppuccin before Bootstrap; adds dark theme overrides and utility classes. |
| app/assets/stylesheets/_catppuccin.scss | Adds Catppuccin palette, Bootstrap variable overrides, and self-hosted font faces. |
| AGENTS.md | Adds pointers to GOTCHAS/DESIGN and documents theme/layout/toast conventions. |
| .reek.yml | Removes Railsboot-specific exclusions now that Railsboot is removed. |
| .cursor/rules/.gitignore | Ignores Tessl-managed Cursor rule files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5
to
+8
| <li class="breadcrumb-item"><%= link_to "Dashboard", root_path %></li> | ||
| <li class="breadcrumb-item"><%= link_to "Admin", admin_index_path %></li> | ||
| <li class="breadcrumb-item"><%= link_to @project.name, @campaign %></li> | ||
| <li class="breadcrumb-item active" aria-current="page">Edit</li> |
Comment on lines
+9
to
+13
| </nav> | ||
| <div class="d-flex justify-content-between align-items-center"> | ||
| <h2 class="mb-0"><%= title @project.name %></h2> | ||
| <div></div> | ||
| </div> |
Comment on lines
+12
to
+16
| <div> | ||
| <% if can? :edit, Attack %> | ||
| <%= link_to edit_project_path(@project), class: "btn btn-primary btn-sm", aria: { label: "Edit project" } do %> | ||
| <%= icon("pencil") %> | ||
| <% end %> |
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
coderabbitai
Bot
removed
documentation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/agents/_form.html.erb (1)
23-35:⚠️ Potential issue | 🟠 MajorCorrect the advanced_configuration implementation assumption and address I18n violations.
The
<fieldset>and<legend>pattern is solid for accessibility, but the verification assumption is off.advanced_configurationuses StoreModel (include StoreModel::Model), notaccepts_nested_attributes_for. Thesimple_fields_forcorrectly integrates with StoreModel for form binding—no changes needed there.Fix the I18n violation: all hints must use translation keys (e.g.,
hint: t(".agent_update_interval_hint")). The hardcoded strings on lines 27, 29, 31, and 33 violate the guideline requiring I18n for all user-visible text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/agents/_form.html.erb` around lines 23 - 35, The form currently passes hard-coded hint strings for the advanced_configuration StoreModel fields; replace each literal hint with I18n lookups using translation keys (e.g. use t(".agent_update_interval_hint"), t(".use_native_hashcat_hint"), t(".backend_device_hint"), t(".opencl_devices_hint") ) so the simple_fields_for :advanced_configuration block continues to bind correctly to the StoreModel but all user-visible hints use translations; add the matching keys to the view locale file under the appropriate scope.
♻️ Duplicate comments (3)
app/views/shared/attack_resource/view_file.html.erb (1)
15-17:⚠️ Potential issue | 🟠 MajorAdd an accessible name to the icon-only download control.
The link is icon-only, so screen readers get an unnamed interactive element. Add
aria-labeland hidden text.Proposed fix
- <%= link_to url_for([:download, `@resource`]), class: "btn btn-primary btn-sm" do %> - <%= icon("download") %> + <%= link_to url_for([:download, `@resource`]), + class: "btn btn-primary btn-sm", + aria: { label: "Download file" } do %> + <span aria-hidden="true"><%= icon("download") %></span> + <span class="visually-hidden">Download file</span> <% end %>As per coding guidelines, "Implement proper ARIA labels in views" and "Support screen readers in views."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/shared/attack_resource/view_file.html.erb` around lines 15 - 17, The icon-only download link (link_to url_for([:download, `@resource`]) with icon("download")) lacks an accessible name; update the link_to call to include an aria-label (e.g., aria-label: "Download file") and add visually hidden text inside the block (a span with an accessible-hidden class or sr-only content) next to icon("download") so screen readers read a descriptive label while visual appearance remains icon-only.app/views/projects/new.html.erb (1)
10-13: 🧹 Nitpick | 🔵 TrivialDrop empty placeholder div in header row.
Line 12 is layout-only dead markup. Remove it and adjust flex alignment.
Suggested fix
- <div class="d-flex justify-content-between align-items-center"> + <div class="d-flex align-items-center"> <h2 class="mb-0"><%= title "New Project" %></h2> - <div></div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/projects/new.html.erb` around lines 10 - 13, Remove the layout-only empty placeholder div and adjust the header flex classes: delete the empty <div></div> that follows the h2 (the header block containing h2 with title "New Project") and change the container's class from "d-flex justify-content-between align-items-center" to "d-flex justify-content-start align-items-center" so the heading stays properly aligned without the dead markup.app/views/projects/_form.html.erb (1)
4-4:⚠️ Potential issue | 🟡 MinorFix error-summary sentence grammar at Line 4.
The heading reads incorrectly for singular counts (“1 error prevent…”). Use an invariant sentence.
Suggested fix
- <h4 class="alert-heading"><%= pluralize(`@project.errors.count`, "error") %> prevent this from saving.</h4> + <h4 class="alert-heading"><%= pluralize(`@project.errors.count`, "error") %> prohibited this project from being saved.</h4>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/projects/_form.html.erb` at line 4, The heading uses pluralize(`@project.errors.count`, "error") but uses the incorrect verb "prevent" causing grammar errors for singular counts; update the H4 line (the heading that builds on pluralize(`@project.errors.count`, "error") and `@project.errors.count`) to use an invariant past-tense verb—e.g., change "prevent" to "prevented" so the sentence reads correctly for both "1 error" and multiple errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/views/attacks/_form.html.erb`:
- Line 4: In the _form.html.erb template update the validation heading that
currently uses pluralize(`@attack.errors.count`, "error") with the fixed verb
tense so it reads correctly for singular and plural; specifically change the
trailing text from "prevent this from saving." to "prevented this from saving."
(the h4 containing pluralize(`@attack.errors.count`, "error")).
In `@app/views/attacks/show.html.erb`:
- Line 7: The view assumes `@attack.campaign` exists and will raise NoMethodError
if nil; update the show view to guard access to `@attack.campaign` by either using
safe navigation when rendering the name and link (e.g., only call name and pass
the campaign to link_to if present) or wrap the breadcrumb line in a conditional
that checks `@attack.campaign.present`? and renders a fallback (or omits the link)
when nil; ensure the same guard covers both the displayed text and the link
target so `@attack.campaign` is never dereferenced when nil.
In `@app/views/campaigns/_form.html.erb`:
- Line 4: The heading currently uses `@campaign.errors.count` with pluralize for
"error" but hardcodes the verb "prevent", causing incorrect grammar when count
== 1; update the <h4> line in app/views/campaigns/_form.html.erb to choose the
verb based on `@campaign.errors.count` (e.g., use a conditional expression on
`@campaign.errors.count` to render "prevents" when count == 1 and "prevent"
otherwise) while preserving the existing pluralize(`@campaign.errors.count`,
"error") output.
In `@app/views/campaigns/new.html.erb`:
- Line 33: The render call passes locals (render "form", campaign: `@campaign`,
hash_lists: `@hash_lists`) but the partial uses instance variables (`@campaign`,
`@hash_lists`), causing inconsistent coupling; either remove the unused locals
from the render call or, preferably, update the partial to accept and use the
locals (campaign and hash_lists) instead of `@campaign/`@hash_lists so the
component consistently relies on its explicit inputs (adjust the partial
template to reference campaign and hash_lists).
In `@app/views/hash_lists/_form.html.erb`:
- Line 5: The heading uses pluralize(`@hash_list.errors.count`, "error") but
always shows the same verb; change the heading to choose the correct verb based
on `@hash_list.errors.count` (e.g., use a conditional on `@hash_list.errors.count`
to render "prevents" for a singular count and "prevent" for plural) so the
sentence reads correctly for 1 error and multiple errors; update the heading
that references `@hash_list.errors.count` to include this conditional verb
selection.
In `@app/views/partials/_navbar.html.erb`:
- Around line 21-37: The Tools dropdown currently renders an empty menu when the
user lacks permission for WordList, MaskList, and RuleList; wrap the entire <li
class="nav-item dropdown"> (the Tools dropdown block) in a single conditional
that checks if the user can read at least one of WordList, MaskList, or RuleList
(e.g., using can? :read, WordList || can? :read, MaskList || can? :read,
RuleList) so the dropdown and its button are omitted entirely when no
permissions exist.
In `@app/views/projects/edit.html.erb`:
- Around line 11-14: The header flex container contains an unnecessary empty
spacer div after the H2 (the <div class="d-flex justify-content-between
align-items-center"> wrapping the <h2 class="mb-0"><%= title `@project.name`
%></h2>), so remove that empty <div></div> node to simplify the markup and let
the flex container rely on its existing children; update the file view template
that renders the project header to delete the empty spacer div.
In `@app/views/shared/attack_resource/_form.html.erb`:
- Line 24: The Cancel link and breadcrumb links use url_for(controller:
controller_name) which inherits the current action; update each occurrence
(e.g., the Cancel link in shared partial _form.html.erb where link_to "Cancel",
url_for(controller: controller_name) is used, and the breadcrumb links in the
same shared partials used by word_lists, mask_lists, rule_lists, and hash_lists)
to explicitly include action: :index (i.e., url_for(controller: controller_name,
action: :index)) so the links always navigate to the index page rather than
retaining new/edit params.
---
Outside diff comments:
In `@app/views/agents/_form.html.erb`:
- Around line 23-35: The form currently passes hard-coded hint strings for the
advanced_configuration StoreModel fields; replace each literal hint with I18n
lookups using translation keys (e.g. use t(".agent_update_interval_hint"),
t(".use_native_hashcat_hint"), t(".backend_device_hint"),
t(".opencl_devices_hint") ) so the simple_fields_for :advanced_configuration
block continues to bind correctly to the StoreModel but all user-visible hints
use translations; add the matching keys to the view locale file under the
appropriate scope.
---
Duplicate comments:
In `@app/views/projects/_form.html.erb`:
- Line 4: The heading uses pluralize(`@project.errors.count`, "error") but uses
the incorrect verb "prevent" causing grammar errors for singular counts; update
the H4 line (the heading that builds on pluralize(`@project.errors.count`,
"error") and `@project.errors.count`) to use an invariant past-tense verb—e.g.,
change "prevent" to "prevented" so the sentence reads correctly for both "1
error" and multiple errors.
In `@app/views/projects/new.html.erb`:
- Around line 10-13: Remove the layout-only empty placeholder div and adjust the
header flex classes: delete the empty <div></div> that follows the h2 (the
header block containing h2 with title "New Project") and change the container's
class from "d-flex justify-content-between align-items-center" to "d-flex
justify-content-start align-items-center" so the heading stays properly aligned
without the dead markup.
In `@app/views/shared/attack_resource/view_file.html.erb`:
- Around line 15-17: The icon-only download link (link_to url_for([:download,
`@resource`]) with icon("download")) lacks an accessible name; update the link_to
call to include an aria-label (e.g., aria-label: "Download file") and add
visually hidden text inside the block (a span with an accessible-hidden class or
sr-only content) next to icon("download") so screen readers read a descriptive
label while visual appearance remains icon-only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c072b762-f739-4b8a-a9ec-1f2a7420d5f6
📒 Files selected for processing (13)
app/views/admin/_user_form.html.erbapp/views/agents/_form.html.erbapp/views/attacks/_form.html.erbapp/views/attacks/show.html.erbapp/views/campaigns/_form.html.erbapp/views/campaigns/new.html.erbapp/views/hash_lists/_form.html.erbapp/views/partials/_navbar.html.erbapp/views/projects/_form.html.erbapp/views/projects/edit.html.erbapp/views/projects/new.html.erbapp/views/shared/attack_resource/_form.html.erbapp/views/shared/attack_resource/view_file.html.erb
| <%= render Railsboot::ErrorsComponent.new(@attack) %> | ||
| <% if @attack.errors.any? %> | ||
| <div class="alert alert-danger" role="alert"> | ||
| <h4 class="alert-heading"><%= pluralize(@attack.errors.count, "error") %> prevent this from saving.</h4> |
Contributor
There was a problem hiding this comment.
Fix singular grammar in validation heading (Line 4).
pluralize can return “1 error”, but the sentence always uses “prevent”. This renders as “1 error prevent…”. Use neutral phrasing or verb agreement.
Suggested patch
- <h4 class="alert-heading"><%= pluralize(`@attack.errors.count`, "error") %> prevent this from saving.</h4>
+ <h4 class="alert-heading">
+ <%= pluralize(`@attack.errors.count`, "error") %> <%= `@attack.errors.count` == 1 ? "prevents" : "prevent" %> this attack from saving.
+ </h4>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/attacks/_form.html.erb` at line 4, In the _form.html.erb template
update the validation heading that currently uses
pluralize(`@attack.errors.count`, "error") with the fixed verb tense so it reads
correctly for singular and plural; specifically change the trailing text from
"prevent this from saving." to "prevented this from saving." (the h4 containing
pluralize(`@attack.errors.count`, "error")).
| <ol class="breadcrumb"> | ||
| <li class="breadcrumb-item"><%= link_to "Dashboard", root_path %></li> | ||
| <li class="breadcrumb-item"><%= link_to "Activity", campaigns_path %></li> | ||
| <li class="breadcrumb-item"><%= link_to @attack.campaign.name, @attack.campaign %></li> |
Contributor
There was a problem hiding this comment.
Nil guard missing for @attack.campaign.
If @attack.campaign is nil (orphaned record, stale data), this line throws NoMethodError. Add a safe navigation or guard.
🛡️ Defensive fix
- <li class="breadcrumb-item"><%= link_to `@attack.campaign.name`, `@attack.campaign` %></li>
+ <li class="breadcrumb-item"><%= link_to `@attack.campaign`&.name || "Unknown", `@attack.campaign` if `@attack.campaign` %></li>Or ensure controller always preloads campaign with a validation/redirect if missing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <li class="breadcrumb-item"><%= link_to @attack.campaign.name, @attack.campaign %></li> | |
| <li class="breadcrumb-item"><%= link_to `@attack.campaign`&.name || "Unknown", `@attack.campaign` if `@attack.campaign` %></li> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/attacks/show.html.erb` at line 7, The view assumes `@attack.campaign`
exists and will raise NoMethodError if nil; update the show view to guard access
to `@attack.campaign` by either using safe navigation when rendering the name and
link (e.g., only call name and pass the campaign to link_to if present) or wrap
the breadcrumb line in a conditional that checks `@attack.campaign.present`? and
renders a fallback (or omits the link) when nil; ensure the same guard covers
both the displayed text and the link target so `@attack.campaign` is never
dereferenced when nil.
| <%= render Railsboot::ErrorsComponent.new(@campaign) %> | ||
| <% if @campaign.errors.any? %> | ||
| <div class="alert alert-danger" role="alert"> | ||
| <h4 class="alert-heading"><%= pluralize(@campaign.errors.count, "error") %> prevent this from saving.</h4> |
Contributor
There was a problem hiding this comment.
Fix singular/plural grammar in the error heading.
Line 4 always uses “prevent”, so singular output reads incorrectly (“1 error prevent…”). Make the verb agreement conditional.
Suggested patch
- <h4 class="alert-heading"><%= pluralize(`@campaign.errors.count`, "error") %> prevent this from saving.</h4>
+ <% error_count = `@campaign.errors.count` %>
+ <h4 class="alert-heading">
+ <%= pluralize(error_count, "error") %> <%= error_count == 1 ? "prevents" : "prevent" %> this from saving.
+ </h4>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <h4 class="alert-heading"><%= pluralize(@campaign.errors.count, "error") %> prevent this from saving.</h4> | |
| <% error_count = `@campaign.errors.count` %> | |
| <h4 class="alert-heading"> | |
| <%= pluralize(error_count, "error") %> <%= error_count == 1 ? "prevents" : "prevent" %> this from saving. | |
| </h4> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/campaigns/_form.html.erb` at line 4, The heading currently uses
`@campaign.errors.count` with pluralize for "error" but hardcodes the verb
"prevent", causing incorrect grammar when count == 1; update the <h4> line in
app/views/campaigns/_form.html.erb to choose the verb based on
`@campaign.errors.count` (e.g., use a conditional expression on
`@campaign.errors.count` to render "prevents" when count == 1 and "prevent"
otherwise) while preserving the existing pluralize(`@campaign.errors.count`,
"error") output.
| </div> | ||
|
|
||
| <% else %> | ||
| <%= render "form", campaign: @campaign, hash_lists: @hash_lists %> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused locals when rendering _form.
Line 33 passes campaign: and hash_lists:, but the partial currently reads instance vars (@campaign, @hash_lists). Keep one pattern to avoid hidden coupling.
Suggested patch
- <%= render "form", campaign: `@campaign`, hash_lists: `@hash_lists` %>
+ <%= render "form" %>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <%= render "form", campaign: @campaign, hash_lists: @hash_lists %> | |
| <%= render "form" %> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/campaigns/new.html.erb` at line 33, The render call passes locals
(render "form", campaign: `@campaign`, hash_lists: `@hash_lists`) but the partial
uses instance variables (`@campaign`, `@hash_lists`), causing inconsistent coupling;
either remove the unused locals from the render call or, preferably, update the
partial to accept and use the locals (campaign and hash_lists) instead of
`@campaign/`@hash_lists so the component consistently relies on its explicit
inputs (adjust the partial template to reference campaign and hash_lists).
| <%= render Railsboot::ErrorsComponent.new(@hash_list) %> | ||
| <% if @hash_list.errors.any? %> | ||
| <div class="alert alert-danger" role="alert"> | ||
| <h4 class="alert-heading"><%= pluralize(@hash_list.errors.count, "error") %> prevent this from saving.</h4> |
Contributor
There was a problem hiding this comment.
Fix verb agreement in the error heading text.
At Line 5, the sentence is grammatically incorrect for singular counts (1 error prevent...). Use a count-aware verb to keep the heading readable.
Proposed patch
- <h4 class="alert-heading"><%= pluralize(`@hash_list.errors.count`, "error") %> prevent this from saving.</h4>
+ <h4 class="alert-heading">
+ <%= pluralize(`@hash_list.errors.count`, "error") %>
+ <%= `@hash_list.errors.count` == 1 ? "prevents" : "prevent" %> this from saving.
+ </h4>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <h4 class="alert-heading"><%= pluralize(@hash_list.errors.count, "error") %> prevent this from saving.</h4> | |
| <h4 class="alert-heading"> | |
| <%= pluralize(`@hash_list.errors.count`, "error") %> | |
| <%= `@hash_list.errors.count` == 1 ? "prevents" : "prevent" %> this from saving. | |
| </h4> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/hash_lists/_form.html.erb` at line 5, The heading uses
pluralize(`@hash_list.errors.count`, "error") but always shows the same verb;
change the heading to choose the correct verb based on `@hash_list.errors.count`
(e.g., use a conditional on `@hash_list.errors.count` to render "prevents" for a
singular count and "prevent" for plural) so the sentence reads correctly for 1
error and multiple errors; update the heading that references
`@hash_list.errors.count` to include this conditional verb selection.
Comment on lines
+21
to
+37
| <li class="nav-item dropdown"> | ||
| <button type="button" class="nav-link dropdown-toggle" data-bs-toggle="dropdown" aria-expanded="false"> | ||
| <%= icon("gear-wide-connected") %> | ||
| Tools | ||
| </a> | ||
| </button> | ||
| <ul class="dropdown-menu"> | ||
| <% if can? :read, WordList %> | ||
| <li><%= link_to "Word Lists", word_lists_path, class: "dropdown-item" %></li> | ||
| <% end %> | ||
| <% if can? :read, MaskList %> | ||
| <li><%= link_to "Mask Lists", mask_lists_path, class: "dropdown-item" %></li> | ||
| <% end %> | ||
| <% if can? :read, RuleList %> | ||
| <li><%= link_to "Rule Lists", rule_lists_path, class: "dropdown-item" %></li> | ||
| <% end %> | ||
| </ul> | ||
| </li> |
Contributor
There was a problem hiding this comment.
Conditionally render Tools dropdown to prevent empty menu.
If a user lacks permission for all three list types, the dropdown renders but contains an empty <ul>. This confuses users and screen readers.
Wrap the entire dropdown in a permission check:
🛡️ Proposed fix
+ <% if can?(:read, WordList) || can?(:read, MaskList) || can?(:read, RuleList) %>
<li class="nav-item dropdown">
<button type="button" class="nav-link dropdown-toggle" data-bs-toggle="dropdown" aria-expanded="false">
<%= icon("gear-wide-connected") %>
Tools
</button>
<ul class="dropdown-menu">
<% if can? :read, WordList %>
<li><%= link_to "Word Lists", word_lists_path, class: "dropdown-item" %></li>
<% end %>
<% if can? :read, MaskList %>
<li><%= link_to "Mask Lists", mask_lists_path, class: "dropdown-item" %></li>
<% end %>
<% if can? :read, RuleList %>
<li><%= link_to "Rule Lists", rule_lists_path, class: "dropdown-item" %></li>
<% end %>
</ul>
</li>
+ <% end %>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/partials/_navbar.html.erb` around lines 21 - 37, The Tools dropdown
currently renders an empty menu when the user lacks permission for WordList,
MaskList, and RuleList; wrap the entire <li class="nav-item dropdown"> (the
Tools dropdown block) in a single conditional that checks if the user can read
at least one of WordList, MaskList, or RuleList (e.g., using can? :read,
WordList || can? :read, MaskList || can? :read, RuleList) so the dropdown and
its button are omitted entirely when no permissions exist.
Comment on lines
+11
to
+14
| <div class="d-flex justify-content-between align-items-center"> | ||
| <h2 class="mb-0"><%= title @project.name %></h2> | ||
| <div></div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove empty spacer div from header container.
Line 13 adds no semantic or functional value. Simplify the flex container.
Suggested fix
- <div class="d-flex justify-content-between align-items-center">
+ <div class="d-flex align-items-center">
<h2 class="mb-0"><%= title `@project.name` %></h2>
- <div></div>
</div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/projects/edit.html.erb` around lines 11 - 14, The header flex
container contains an unnecessary empty spacer div after the H2 (the <div
class="d-flex justify-content-between align-items-center"> wrapping the <h2
class="mb-0"><%= title `@project.name` %></h2>), so remove that empty <div></div>
node to simplify the markup and let the flex container rely on its existing
children; update the file view template that renders the project header to
delete the empty spacer div.
| <div class="form-actions"> | ||
| <%= f.button :submit, class: "btn btn-primary" %> | ||
| <%= render Railsboot::ButtonComponent.new(tag: "a", href: url_for(controller: controller_name), variant: "danger", text: "Cancel") %> | ||
| <%= link_to "Cancel", url_for(controller: controller_name), class: "btn btn-secondary" %> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify potential inherited-action cancel links in views.
rg -n -C2 --type-add 'erb:*.erb' --type erb 'link_to\s+["'"'"']Cancel["'"'"'].*url_for\(controller:\s*controller_name\)' app/views
# Also find all controller-only url_for usages for manual audit.
rg -n --type-add 'erb:*.erb' --type erb 'url_for\(controller:\s*controller_name\)' app/viewsRepository: unclesp1d3r/CipherSwarm
Length of output: 1219
🏁 Script executed:
#!/bin/bash
# Check the controller structure and verify if index action exists
cd app
find . -path "*/controllers/*" -name "*attack*.rb" | head -5
# Get more context about the form partial and its usage
echo "=== Checking form partial context ==="
head -30 views/shared/attack_resource/_form.html.erb
# Check if this form is called from new/edit templates
echo "=== Looking for form usage in new/edit ==="
rg -l "_form" views/shared/attack_resource/ | xargs cat
# Check the controller to understand routing
echo "=== Checking for attack_resource controller ==="
find . -name "*attack*" -type f | grep -i controllerRepository: unclesp1d3r/CipherSwarm
Length of output: 2629
🏁 Script executed:
#!/bin/bash
# Verify the actual behavior - check where this partial is included
rg -B5 -A5 'render.*_form' app/views/
# Check if url_for with only controller truly inherits action
echo "=== Checking Rails url_for behavior documentation ==="
rg -i 'action.*url_for|url_for.*action' app/Repository: unclesp1d3r/CipherSwarm
Length of output: 2196
🏁 Script executed:
#!/bin/bash
# Find the actual controllers being referenced to verify index action exists
echo "=== Find all controllers with attack_resource ==="
fd -e rb -p 'controllers' | head -20
# Check what action/id context the form is typically called in
rg -n 'def new|def edit|def create|def update' app/controllers/ | grep -i attackRepository: unclesp1d3r/CipherSwarm
Length of output: 1370
🏁 Script executed:
#!/bin/bash
# Check if index action exists in attacks controllers
echo "=== Checking for index actions in attacks controllers ==="
rg -A1 "def index" app/controllers/attacks_controller.rb app/controllers/admin/attacks_controller.rb
# Check all locations where this shared partial is rendered
echo "=== Finding all render calls for shared attack_resource partials ==="
rg -r "shared/attack_resource" app/views/ --type erb
# Verify url_for behavior - check if other partials use implicit controller
echo "=== Checking other url_for patterns in shared partials ==="
rg -n "url_for\(" app/views/shared/ --type erb | head -20Repository: unclesp1d3r/CipherSwarm
Length of output: 2561
🏁 Script executed:
#!/bin/bash
# Check controller inheritance
echo "=== Checking attacks_controller parent class ==="
head -20 app/controllers/attacks_controller.rb
# Check if there's a base class with index
echo "=== Checking for index in base controllers ==="
rg -n "def index" app/controllers/application_controller.rb
# Verify the actual routes - maybe index isn't used
echo "=== Checking config/routes.rb for attacks routing ==="
rg -i "attacks" config/routes.rb
# Check all controllers that might use this shared partial
echo "=== Looking for which controllers render attack_resource partials ==="
rg -B5 "shared/attack_resource" app/views --type erb | grep -E "app/views/(admin|attacks|hash|mask|rule|word)" | head -20Repository: unclesp1d3r/CipherSwarm
Length of output: 5466
🏁 Script executed:
#!/bin/bash
# Critical check - does attacks controller have index at all?
echo "=== Full attacks_controller.rb to see all actions ==="
wc -l app/controllers/attacks_controller.rb
grep "def " app/controllers/attacks_controller.rb
# Check other controllers that use this shared partial
echo "=== Check word_lists, mask_lists, rule_lists controllers ==="
grep "def " app/controllers/word_lists_controller.rb 2>/dev/null || echo "word_lists_controller not found"
grep "def " app/controllers/mask_lists_controller.rb 2>/dev/null || echo "mask_lists_controller not found"
grep "def " app/controllers/rule_lists_controller.rb 2>/dev/null || echo "rule_lists_controller not found"
# Find these controllers
echo "=== Finding list controllers ==="
fd -e rb controllers | xargs grep -l "def index"Repository: unclesp1d3r/CipherSwarm
Length of output: 974
🏁 Script executed:
#!/bin/bash
# This is critical - attacks has NO index action but shares a partial with controllers that DO
# Check the routing and nested resource structure
echo "=== Check if attacks are nested resources ==="
grep -A10 "resources :campaigns" config/routes.rb
echo "=== Check what the cancel should navigate to for attacks ==="
# For nested attacks, cancel should go back to campaign, not campaign_attacks#index (which doesn't exist)
cat app/controllers/attacks_controller.rb | grep -A20 "def new"
echo "=== Check how campaigns show lists attacks ==="
grep -n "attack" app/views/campaigns/show.html.erb | head -10
# Check if there's a redirect pattern for attacks after cancel
echo "=== Look for how form redirects are handled ==="
grep -n "redirect\|campaigns_path\|campaign_path" app/controllers/attacks_controller.rbRepository: unclesp1d3r/CipherSwarm
Length of output: 2682
🏁 Script executed:
#!/bin/bash
# CRITICAL: Check if attacks actually use the shared _form partial
echo "=== Verify if attacks views use shared _form ==="
ls -la app/views/attacks/
cat app/views/attacks/new.html.erb
cat app/views/attacks/edit.html.erb
echo "=== Compare with word_lists ==="
ls -la app/views/word_lists/
cat app/views/word_lists/new.html.erb | head -20Repository: unclesp1d3r/CipherSwarm
Length of output: 3216
🏁 Script executed:
#!/bin/bash
# CRITICAL DISCOVERY: Verify that shared_attack_resource partials are NOT used by attacks
echo "=== Verify file location and usage ==="
cat app/views/shared/attack_resource/_form.html.erb | head -5
cat app/views/attacks/_form.html.erb | head -5
echo "=== Check all files in shared/attack_resource ==="
ls -1 app/views/shared/attack_resource/
echo "=== Confirm word_lists uses shared partial and HAS index ==="
grep "def index" app/controllers/word_lists_controller.rb
grep -n "render.*shared/attack_resource/_form" app/views/word_lists/
echo "=== Verify all affected controllers have index action ==="
for ctrl in word_lists mask_lists rule_lists hash_lists; do
echo "=== $ctrl ==="
grep "def index" app/controllers/${ctrl}_controller.rb 2>/dev/null || echo "NO INDEX"
doneRepository: unclesp1d3r/CipherSwarm
Length of output: 1134
🏁 Script executed:
#!/bin/bash
# Perfect! Now verify the complete scope - check the breadcrumbs in shared partial
echo "=== Check all url_for patterns in shared/attack_resource partials ==="
grep -n "url_for" app/views/shared/attack_resource/*.html.erb
echo "=== Confirm these are the ONLY controllers affected ==="
rg "shared/attack_resource" app/views --files-with-matches
echo "=== Verify the breadcrumb issue - what does controller_name resolve to in new/edit context ==="
cat app/views/shared/attack_resource/_new.html.erbRepository: unclesp1d3r/CipherSwarm
Length of output: 3874
🏁 Script executed:
#!/bin/bash
# Final verification - test the exact rails behavior with url_for inheritance
echo "=== Confirm Rails url_for inherits action when not specified ==="
# Check Rails docs pattern - url_for with only controller inherits current action
rg -i "url_for.*action\|url_for.*index" config/ app/ --type rb | head -10
echo "=== Check if any test files validate this behavior ==="
fd -e rb spec/ test/ 2>/dev/null | xargs grep -l "url_for.*controller:" 2>/dev/null | head -5
echo "=== Final scope: ALL affected locations ==="
echo "Cancel button: _form.html.erb:24"
echo "Breadcrumb in _new.html.erb:7"
echo "Breadcrumb in _edit.html.erb:7"
echo "Breadcrumb in _show.html.erb:8"
echo ""
echo "Affected controllers: word_lists, mask_lists, rule_lists, hash_lists"
echo "All have: def index ✓"Repository: unclesp1d3r/CipherSwarm
Length of output: 491
Fix url_for(controller: controller_name) to explicitly specify action: :index across breadcrumb and form action links.
url_for without an explicit action inherits from current params, so Cancel buttons and breadcrumb links in new/edit context will navigate back to new/edit instead of index. Affects 4 locations in shared partials used by word_lists, mask_lists, rule_lists, and hash_lists controllers.
Required fixes
# app/views/shared/attack_resource/_form.html.erb:24
- <%= link_to "Cancel", url_for(controller: controller_name), class: "btn btn-secondary" %>
+ <%= link_to "Cancel", url_for(action: :index), class: "btn btn-secondary" %>
# app/views/shared/attack_resource/_new.html.erb:7
- <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(controller: controller_name) %></li>
+ <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(action: :index) %></li>
# app/views/shared/attack_resource/_edit.html.erb:7
- <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(controller: controller_name) %></li>
+ <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(action: :index) %></li>
# app/views/shared/attack_resource/_show.html.erb:8
- <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(controller: controller_name) %></li>
+ <li class="breadcrumb-item"><%= link_to resource.class.model_name.human.pluralize, url_for(action: :index) %></li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <%= link_to "Cancel", url_for(controller: controller_name), class: "btn btn-secondary" %> | |
| <%= link_to "Cancel", url_for(action: :index), class: "btn btn-secondary" %> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/shared/attack_resource/_form.html.erb` at line 24, The Cancel link
and breadcrumb links use url_for(controller: controller_name) which inherits the
current action; update each occurrence (e.g., the Cancel link in shared partial
_form.html.erb where link_to "Cancel", url_for(controller: controller_name) is
used, and the breadcrumb links in the same shared partials used by word_lists,
mask_lists, rule_lists, and hash_lists) to explicitly include action: :index
(i.e., url_for(controller: controller_name, action: :index)) so the links always
navigate to the index page rather than retaining new/edit params.
Owner
Author
|
@Mergifyio queue |
Contributor
Merge Queue Status
This pull request spent 9 seconds in the queue, including 1 second running CI. Required conditions to merge
ReasonPull request #706 has been dequeued, merge conditions unmatch:
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. |
58 tasks
unclesp1d3r
deleted the
705-feat-frontend-audit-accessibility-responsive-design-theming-and-performance-improvements
branch
mergify Bot
added a commit
that referenced
this pull request
Mar 14, 2026
Update documentation for #706 Introduces comprehensive documentation for the CipherSwarm V2 upgrade, detailing the platform transformation from a functional tool to an enterprise-grade distributed password cracking platform. Covers upgrade philosophy, target user personas, eight core requirements with technical implementation details, enhanced technology stack (Rails 8, Bootstrap 5, Hotwire), a 26-week implementation timeline across six milestones, success metrics, and migration strategy with backward compatibility guarantees. _Generated by [Dosu](https://dosu.dev)_ --------- Co-authored-by: dosubot[bot] <131922026+dosubot[bot]@users.noreply.github.com> Co-authored-by: UncleSp1d3r <unclespider@protonmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.
Summary
Comprehensive frontend audit and overhaul addressing accessibility, theming, responsive design, and component architecture. Removes the Railsboot abstraction layer (117 files, ~1,250 lines deleted), establishes the Catppuccin Macchiato dark theme with DarkViolet accents, fixes 15+ accessibility issues, and adds self-hosted typography — all while keeping the net diff negative (-1,246 lines).
209 files changed · 1,639 additions · 2,885 deletions · net −1,246 lines
What Changed
Accessibility (WCAG 2.1 AA)
<body>, targeting#main-content<a href="#">to<button type="button">(no scroll-to-top, correct semantics)aria-labelattributes<ul>getsaria-label="Main navigation"aria-selected/tabindexmanagementtext-muted→text-body-secondaryacross all views (Bootstrap 5.3 semantic class)style="font-size:64px"to.empty-state-iconCSS class<h1>Catppuccin Macchiato Theme
_catppuccin.scsswith full Macchiato palette (26 colors) + Bootstrap variable overrides$ctp-violet: #a855f7(DarkViolet lightened for dark-mode contrast)[data-bs-theme="dark"]selector for proper Bootstrap 5.3 integrationTypography (Self-Hosted, Air-Gap Safe)
@fontsourcepackages, woff2 files copied toapp/assets/builds/by build scriptResponsive Design
<div class="table-responsive">(attacks index, hash lists index, hash list show)_sidebar_navbar_items.html.erbRailsboot Component Removal
app/components/railsboot/directory removed.reek.ymlcleaned of all Railsboot exclusions (110 lines removed)Documentation
DESIGN.md— new file documenting user personas, brand personality, aesthetic direction, design principlesAGENTS.md— added Catppuccin theme section, layout grid patterns, toast notification rules, font stack docsGOTCHAS.md— added Frontend & Accessibility section (navbar buttons, z-index utilities, ARIA, Turbo morph, Propshaft digest caching,assets:clobberdestructive behavior)Tooling
.coderabbit.yamlfor automated code reviewtessl.json+.tessl/for GitHub Action monitoring.cursor/rules/.gitignorefor Tessl-managed filesWhy These Changes
Issue #705 — the frontend had accumulated tech debt from the Railsboot component layer (an abstraction over Bootstrap that made customization harder), lacked a cohesive dark theme, had numerous WCAG violations, and used CDN-dependent fonts unsuitable for air-gapped deployments. This PR addresses all of these in a single coordinated effort.
Type of Change
How Has This Been Tested?
just testpasses (no view logic changed, only presentation)autohide?behavior (danger=false, others=true)Breaking Changes
None. All changes are presentational — no API, model, controller, or routing changes.
Review Checklist
General
Accessibility
text-mutedusage (replaced withtext-body-secondary)Theming
@import "bootstrap"[data-bs-theme="dark"]used for component-level overridesTypography
font-display: swapon all@font-facedeclarationsResponsive
table-responsiveCleanup
.reek.ymlexclusions removedKnown Remaining Items (Low Priority)
btn-groupinmask_lists/_form.html.erb:14— cosmetic, no functional impact🤖 Generated with Claude Code