[Feat] Server Security#1143
Merged
saeedvaziry merged 6 commits intoJun 4, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds server security controls (SSH password/root login), auto-update scheduling, Fail2ban service implementation and orchestration, a Security dashboard UI, controller endpoints, queued workers to apply/detect changes, DB migration/API schema updates, and comprehensive feature tests. ChangesServer Security Management
Sequence DiagramsequenceDiagram
participant Controller as SecurityController
participant Action as ManageRootLogin
participant Job as ApplyRootLoginJob
participant Security as SSH/OS/Security
participant Server as Server Model
participant Socket as SocketEvent
Controller->>Action: update(server, {enabled: false})
Action->>Action: validate & guard
Action->>Server: refresh & jsonUpdate(UPDATING)
Action->>Job: dispatch(server,false) on ssh queue
Job->>Security: setRootLogin(false) (run templates over SSH)
Security-->>Job: effective setting result
Job->>Server: writeState(READY, detected)
Job->>Socket: broadcast security.updated {server_id}
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Actions/Server/Security/CheckSecurityState.php`:
- Line 12: DetectSecurityJob is being run synchronously via
DetectSecurityJob::dispatchSync($server), which can block the request; change it
to queued dispatch so the job runs asynchronously (e.g., use
DetectSecurityJob::dispatch($server) or dispatch(new DetectSecurityJob($server))
and ensure it goes to the queue), remove blocking call and let the UI update via
socket events instead. Ensure any required job/queue configuration (implements
ShouldQueue, queue connection) is present on DetectSecurityJob so it is actually
queued.
In `@app/Actions/Server/Security/ConfigureFail2ban.php`:
- Around line 26-35: The update to $service->type_data and the subsequent
dispatch of ConfigureFail2banJob must be atomic: wrap the $service->type_data
assignment, $service->save(), and dispatch(new
ConfigureFail2banJob($service))->onQueue('ssh') inside a database transaction
(use DB::transaction or DB::beginTransaction/commit/rollBack) so that if
dispatching the job fails the save is rolled back; locate these calls in
ConfigureFail2ban.php and perform the save and dispatch within the same
try/catch transaction block, rethrow or handle errors after rolling back.
In `@app/Actions/Server/Security/ManageRootLogin.php`:
- Around line 24-28: The current check in ManageRootLogin.php always throws a
ValidationException when $server->getSshUser() === 'root', blocking even
enable=true updates; change the guard to only reject when the request is trying
to disable root login (i.e. when the incoming "enabled" value is false/boolean
falsey). Update the conditional around ValidationException::withMessages to
combine $server->getSshUser() === 'root' AND the parsed incoming enabled value
(use the request or method param that carries "enabled"), so the error is thrown
only on disable attempts and leaves enable=true requests allowed.
In `@app/Jobs/Server/Security/ApplyRootLoginJob.php`:
- Around line 25-31: The current guard in ApplyRootLoginJob.php checks
$this->server->getSshUser() === 'root' and blocks both enable and disable
attempts; change it so it only blocks disable requests: detect the requested
action (e.g. the job's payload flag such as $this->request->enable or
$this->enable / $this->data['enable']) and only run the
writeState(ServerControlStatus::FAILED),
ServerLog::log('disable-root-login-failed', ...) and return when the SSH user is
'root' AND the request is to disable root login; keep enable flows unchanged so
enabling root login succeeds.
In `@app/Services/Fail2ban/Fail2ban.php`:
- Around line 128-135: The version() method in Fail2ban calls
$this->service->server->ssh()->exec(...) which may throw SSHError; update the
PHPDoc for the version() method to include a `@throws` SSHError annotation (or
fully-qualified \SSHError) so the docblock reflects the possible exception.
Ensure the SSHError symbol used in the docblock matches the actual exception
class name/namespace used by the ssh() implementation (adjust the docblock
import/namespace if needed).
In `@app/SSH/OS/Security.php`:
- Line 23: The current return expression in the Security class that does:
str($result)->after('VITO_PASSWORD_AUTH:')->trim()->startsWith('yes') treats
missing/unparsable marker as false; change it to fail-closed by first detecting
whether the marker and a non-empty value exist and return true if the marker is
missing or the extracted value is empty, otherwise return the boolean of
startsWith('yes'). Update the method in Security.php to explicitly check for
presence/emptiness of the after('VITO_PASSWORD_AUTH:')->trim() result and only
call startsWith('yes') when a value exists; if not present, return true
(password auth enabled).
In `@resources/js/pages/security/components/schedule-builder.tsx`:
- Around line 17-31: parseSchedule and buildSchedule currently allow
out-of-range hours/minutes and invalid day-of-week tokens to propagate; update
both to normalize and validate values: in parseSchedule parse m and h as
integers, clamp h to 0–23 and m to 0–59, format time with padStart after
clamping, and validate dow so only '*' or a numeric string between 1 and 7 is
accepted (fall back to '1' if invalid); in buildSchedule keep the existing
clamping for hh/mm but also validate the incoming weekday (use '*' for daily,
and if weekly ensure weekday is numeric 1–7 else default to '1') so the returned
cron is always well-formed (reference functions parseSchedule and
buildSchedule).
In `@resources/js/pages/security/index.tsx`:
- Around line 82-85: Replace the bare onClick submit with proper form submit
semantics: wrap the editable fields in a <form> that has an onSubmit handler
which calls e.preventDefault() and then invokes the existing save function
(referencing the save function and the useForm instance named form), give the
form a unique id, and change the Button to a submit control by removing onClick
and setting type="submit" form="<that-id>" (keeping disabled={form.processing}
and the LoaderCircleIcon rendering). Ensure the new onSubmit uses the form id
and prevents default before calling save so the existing save logic and
form.processing state are preserved.
- Around line 334-336: The handler currently triggers router.reload for any
event matching event.type?.startsWith('service.'), causing reloads from other
servers; update the condition to also verify the event is for the current server
(e.g. compare event.serverId or event.data.serverId to the local server id
variable such as currentServer?.id or serverId) before calling router.reload({
only: ['fail2ban', 'firewall', 'score'] }); keep the existing service.* type
check and use safe optional chaining when reading event.serverId so only events
for the active server trigger the reload.
In `@resources/js/types/server.d.ts`:
- Line 26: The Server type's auto_update_schedule property is currently declared
as a string but the API can return null; update the Server type declaration so
auto_update_schedule allows null (e.g., change its type to include null) to
match the backend resource and avoid runtime null-handling bugs—locate the
Server interface/type and modify the auto_update_schedule field accordingly.
In `@resources/views/ssh/security/disable-password-auth.blade.php`:
- Around line 18-22: The check only inspects sshd's effective
"passwordauthentication" value (EFFECTIVE) but must also verify
"kbdinteractiveauthentication" to ensure keyboard-interactive auth is disabled;
update the logic around the sshd -T parsing (the command assigning EFFECTIVE) to
also capture the effective kbdinteractiveauthentication value (e.g.,
KBD_EFFECTIVE) by extracting the respective keys from sshd -T output, then
require both EFFECTIVE and KBD_EFFECTIVE equal "no" before declaring success; if
either is not "no", emit a single error message that includes both effective
values (referencing the variable names EFFECTIVE and KBD_EFFECTIVE and the sshd
-T extraction) and exit 1.
In `@resources/views/ssh/security/enable-password-auth.blade.php`:
- Around line 8-13: The current check uses "sshd -t" which only validates syntax
but not effective settings; after verifying syntax (sshd -t) and before calling
"sudo systemctl reload ssh", run "sshd -T" and inspect the effective directive
(e.g., grep -i 'passwordauthentication') to ensure PasswordAuthentication is set
to "yes" (and other related directives like PermitEmptyPasswords if needed); if
the effective config does not reflect the intended setting, log an error and
exit nonzero instead of reloading, otherwise proceed to "sudo systemctl reload
ssh" and return success.
In `@resources/views/ssh/security/enable-root-login.blade.php`:
- Around line 8-13: After the syntax check with "sudo sshd -t" also verify the
effective PermitRootLogin value using "sudo sshd -T" (which expands included
files and precedence) and assert the returned "permitrootlogin" equals the
expected policy; if it does not match, print a clear VITO_SSH_ERROR including
the effective value and exit non‑zero instead of reloading. Place this check
between the sshd -t block and the "sudo systemctl reload ssh" call, referencing
"sshd -t", "sshd -T" and the "PermitRootLogin"/"permitrootlogin" setting.
In `@tests/Feature/SecurityTest.php`:
- Around line 295-312: The test calls travelTo(...) to freeze time but never
resets it, causing clock leakage; wrap the time-dependent section (the
travelTo(...) call, the server setup, $this->artisan('servers:auto-update') call
and assertions) in a try/finally and call travelBack() in the finally block (or
at minimum call travelBack() after the assertions) so global test time is
restored; locate the travelTo(...) usage in SecurityTest
(tests/Feature/SecurityTest.php) and add travelBack() to restore time around the
artisan call and Queue/Assert checks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6c7c77a-4ab6-47f6-aaed-399814c184d4
📒 Files selected for processing (41)
app/Actions/Server/Security/CheckSecurityState.phpapp/Actions/Server/Security/ConfigureFail2ban.phpapp/Actions/Server/Security/ManageAutoUpdate.phpapp/Actions/Server/Security/ManagePasswordAuth.phpapp/Actions/Server/Security/ManageRootLogin.phpapp/Console/Commands/RunServerAutoUpdatesCommand.phpapp/Console/Kernel.phpapp/Enums/SecurityControlStatus.phpapp/Http/Controllers/ConsoleController.phpapp/Http/Controllers/SecurityController.phpapp/Http/Resources/ServerResource.phpapp/Jobs/Server/Security/ApplyPasswordAuthJob.phpapp/Jobs/Server/Security/ApplyRootLoginJob.phpapp/Jobs/Server/Security/ConfigureFail2banJob.phpapp/Jobs/Server/Security/DetectSecurityJob.phpapp/Models/Server.phpapp/Providers/ServiceTypeServiceProvider.phpapp/SSH/OS/Security.phpapp/Services/Fail2ban/Fail2ban.phpdatabase/migrations/2026_06_03_234007_add_auto_update_schedule_to_servers_table.phppublic/api-docs/openapi/schemas/Server.yamlresources/js/components/app-sidebar.tsxresources/js/components/dialogs/registry.tsresources/js/layouts/server/layout.tsxresources/js/pages/security/components/fail2ban-form.tsxresources/js/pages/security/components/schedule-builder.tsxresources/js/pages/security/index.tsxresources/js/types/security.d.tsresources/js/types/server.d.tsresources/views/ssh/os/upgrade.blade.phpresources/views/ssh/security/disable-password-auth.blade.phpresources/views/ssh/security/disable-root-login.blade.phpresources/views/ssh/security/enable-password-auth.blade.phpresources/views/ssh/security/enable-root-login.blade.phpresources/views/ssh/security/password-auth-status.blade.phpresources/views/ssh/security/root-login-status.blade.phpresources/views/ssh/services/fail2ban/configure.blade.phpresources/views/ssh/services/fail2ban/install.blade.phpresources/views/ssh/services/fail2ban/jail.blade.phpresources/views/ssh/services/fail2ban/uninstall.blade.phptests/Feature/SecurityTest.php
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/pages/security/index.tsx`:
- Around line 368-369: The Tailwind class on the warning icon is using the
invalid suffix-important form; update the JSX for TriangleAlertIcon (inside the
Alert component) to use a valid Tailwind class such as "text-warning" or, if you
need the !important variant, "!text-warning" instead of "text-warning!" so the
utility will apply correctly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 25e441ce-9dcf-4366-ad7f-7156650a7489
📒 Files selected for processing (10)
app/Actions/Server/Security/CheckSecurityState.phpapp/Actions/Server/Security/ManageRootLogin.phpapp/Jobs/Server/Security/ApplyRootLoginJob.phpapp/Jobs/Server/Security/DetectSecurityJob.phpapp/SSH/OS/Security.phpresources/js/pages/security/index.tsxresources/js/types/server.d.tsresources/views/ssh/security/disable-password-auth.blade.phpresources/views/ssh/security/enable-password-auth.blade.phpresources/views/ssh/security/enable-root-login.blade.php
🚧 Files skipped from review as they are similar to previous changes (6)
- app/Actions/Server/Security/CheckSecurityState.php
- resources/views/ssh/security/enable-password-auth.blade.php
- app/Jobs/Server/Security/DetectSecurityJob.php
- app/Actions/Server/Security/ManageRootLogin.php
- app/SSH/OS/Security.php
- app/Jobs/Server/Security/ApplyRootLoginJob.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
saeedvaziry
approved these changes
Jun 4, 2026
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.
This pull request introduces a comprehensive server security management feature, adding new actions, controllers, jobs, and supporting enums to handle various security controls such as automatic updates, SSH password authentication, root login, and Fail2ban configuration. It also updates existing resources and command scheduling to support these features.
Summary by CodeRabbit