fix: Critical and high audit findings (#81) #83

Merged

jmiller merged 1 commits from fix/audit-critical-high into main 2026-06-21 23:10:20 +00:00

Conversation 0 Commits 1 Files Changed 9

+94 -20

jmiller commented 2026-06-21 23:09:55 +00:00

Owner

Copy Link

Copy Source

Summary

Fixes all critical and high severity issues from the full codebase audit (#81).

Closes #71, closes #76, closes #77, closes #78, closes #79, closes #80. Ref #72.

Fixes

#	Severity	Fix
#71	CRITICAL	RestoreCommand passed filepath as record ID — now passes $recordId
#72	CRITICAL	JpaUnarchiver path traversal — rejects ../ and validates realpath() boundary
#76	HIGH	BackupTable::delete() now deletes DB row before file — no data loss on DB failure
#77	HIGH	RestoreEngine staging path sanitized with preg_replace
#78	HIGH	API profiles() masks sensitive fields (***) instead of exposing credentials
#79	HIGH	Webcron handler adds return after sendJsonResponse() auth rejections
#80	MEDIUM	loadFormData() casts array to object — fixes TypeError on PHP 8.x
—	HIGH	PreflightCheck::run() wrapped in try-catch for DB exceptions
—	HIGH	mkdir() failure includes actual error reason
—	MEDIUM	Unresolved placeholders generate warning instead of silent return

Test Plan

• CLI restore: bin/joomla mokosuitebackup:restore 1 works correctly
• JPA restore with normal archive works, crafted ../ paths are rejected
• Deleting a backup record: file deleted only after DB row removed
• API GET profiles: credentials show *** not real values
• Webcron with invalid secret returns 403 and does not run backup
• Profile/Backup edit after validation failure doesn't crash (TypeError fix)
• Preflight with DB down returns clean error, not stack trace

## Summary Fixes all critical and high severity issues from the full codebase audit (#81). Closes #71, closes #76, closes #77, closes #78, closes #79, closes #80. Ref #72. ## Fixes | # | Severity | Fix | |---|----------|-----| | #71 | CRITICAL | `RestoreCommand` passed filepath as record ID — now passes `$recordId` | | #72 | CRITICAL | `JpaUnarchiver` path traversal — rejects `../` and validates `realpath()` boundary | | #76 | HIGH | `BackupTable::delete()` now deletes DB row before file — no data loss on DB failure | | #77 | HIGH | `RestoreEngine` staging path sanitized with `preg_replace` | | #78 | HIGH | API `profiles()` masks sensitive fields (`***`) instead of exposing credentials | | #79 | HIGH | Webcron handler adds `return` after `sendJsonResponse()` auth rejections | | #80 | MEDIUM | `loadFormData()` casts array to object — fixes `TypeError` on PHP 8.x | | — | HIGH | `PreflightCheck::run()` wrapped in try-catch for DB exceptions | | — | HIGH | `mkdir()` failure includes actual error reason | | — | MEDIUM | Unresolved placeholders generate warning instead of silent return | ## Test Plan - [ ] CLI restore: `bin/joomla mokosuitebackup:restore 1` works correctly - [ ] JPA restore with normal archive works, crafted `../` paths are rejected - [ ] Deleting a backup record: file deleted only after DB row removed - [ ] API GET profiles: credentials show `***` not real values - [ ] Webcron with invalid secret returns 403 and does not run backup - [ ] Profile/Backup edit after validation failure doesn't crash (TypeError fix) - [ ] Preflight with DB down returns clean error, not stack trace

jmiller added the component: enginecomponent: admincomponent: api labels 2026-06-21 23:09:55 +00:00

jmiller added 1 commit 2026-06-21 23:09:56 +00:00

fix: critical and high audit findings (#81) ... 2395a4eabc

Fixes all critical and high severity issues from the codebase audit:

CRITICAL:
- #71: RestoreCommand passed wrong args to RestoreEngine (filepath
  instead of record ID) — CLI restore was completely broken
- #72: JpaUnarchiver path traversal — added traversal rejection and
  realpath boundary check to prevent writes outside staging dir
- #77: RestoreEngine staging path sanitized — $record->tag stripped
  of non-alphanumeric characters

HIGH:
- #75: (noted, AkeebaImporter unserialize needs separate refactor)
- #76: BackupTable now deletes DB row before file — prevents data
  loss if DB delete fails
- #78: API profiles endpoint now masks sensitive fields (passwords,
  keys, tokens) with '***'
- #79: Webcron handler adds return after sendJsonResponse — prevents
  execution falling through on non-terminal close()
- #80: BackupModel/ProfileModel loadFormData() now casts array to
  object — prevents TypeError on PHP 8.x form state restore

PREFLIGHT HARDENING:
- PreflightCheck::run() wrapped in try-catch for DB exceptions
- mkdir() failure now includes actual error reason
- Unresolved placeholders generate a warning instead of silent return

Closes #71, closes #76, closes #77, closes #78, closes #79, closes #80
Ref #72, ref #81

jmiller merged commit d939d8c9d7 into main 2026-06-21 23:10:20 +00:00

jmiller deleted branch fix/audit-critical-high 2026-06-21 23:10:24 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

component: admin

Admin UI and views

component: api

REST API

component: engine

Backup/restore engine

component: remote

Remote storage (FTP/GDrive)

component: scheduler

Scheduled tasks

No labels component: admin component: api component: engine

Priority -

Type -

Milestone

No items

No Milestone

Assignees

Clear assignees

jmiller (Jonathan Miller)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoSuiteBackup#83