[audit] Security, code quality, missing tests, error handling #4

New issue

Open

opened 2026-03-22 16:41:19 +00:00 by Virgil · 5 comments

Virgil commented

2026-03-22 16:41:19 +00:00

Member

Full audit:

Security: SQL injection, XSS, CSRF bypass, mass assignment, path traversal, insecure deserialization
Code quality: missing strict_types, missing type hints, missing return types
Missing tests: modules/actions without Pest test coverage
Error handling: swallowed exceptions, bare try/catch, missing validation
UK English: American spellings (color→colour, organization→organisation, center→centre)
Coding standards: PSR-12 compliance, Action pattern usage, BelongsToWorkspace scoping
Missing SPDX licence headers (EUPL-1.2)

Report all findings with severity and file:line. Do NOT fix.

Full audit: 1. Security: SQL injection, XSS, CSRF bypass, mass assignment, path traversal, insecure deserialization 2. Code quality: missing strict_types, missing type hints, missing return types 3. Missing tests: modules/actions without Pest test coverage 4. Error handling: swallowed exceptions, bare try/catch, missing validation 5. UK English: American spellings (color→colour, organization→organisation, center→centre) 6. Coding standards: PSR-12 compliance, Action pattern usage, BelongsToWorkspace scoping 7. Missing SPDX licence headers (EUPL-1.2) Report all findings with severity and file:line. Do NOT fix.

Virgil commented

2026-03-22 19:59:01 +00:00

Author

Member

Codex Audit Findings

CRITICAL (1)

Webhooks accept unsigned requests — missing signatures not rejected (WebhookController.php:76, WebhookReceiverService.php:32)

HIGH (3)

Public webhook test endpoint leaks UUID/vendor/secret-presence without auth (api.php:31, WebhookController.php:266)
Digest emails fail — links to non-existent route (SendUptelligenceDigest.php:81 vs admin.php:23)
Webhook creation validates against wrong table — vendors instead of uptelligence_vendors (WebhookManager.php:135)

## Codex Audit Findings ### CRITICAL (1) 1. Webhooks accept unsigned requests — missing signatures not rejected (WebhookController.php:76, WebhookReceiverService.php:32) ### HIGH (3) 2. Public webhook test endpoint leaks UUID/vendor/secret-presence without auth (api.php:31, WebhookController.php:266) 3. Digest emails fail — links to non-existent route (SendUptelligenceDigest.php:81 vs admin.php:23) 4. Webhook creation validates against wrong table — vendors instead of uptelligence_vendors (WebhookManager.php:135)

Virgil commented

2026-03-23 06:36:30 +00:00

Author

Member

Fix Applied

Commit 95e1905: fix(uptelligence): resolve issue 4 critical and high findings

## Fix Applied Commit 95e1905: fix(uptelligence): resolve issue 4 critical and high findings

Virgil commented

2026-03-23 08:53:42 +00:00

Author

Member

Verification: CONDITIONAL PASS

Fixes correct but test coverage insufficient — signature regression test bypasses routing/middleware, no test for authenticated non-Hades access or disabled/circuit-broken /test. Needs better test coverage.

## Verification: CONDITIONAL PASS Fixes correct but test coverage insufficient — signature regression test bypasses routing/middleware, no test for authenticated non-Hades access or disabled/circuit-broken /test. Needs better test coverage.

Virgil commented

2026-03-23 09:31:40 +00:00

Author

Member

Fix Round 2

Commit cf80299: fix(uptelligence): harden webhook test endpoint coverage

Signature validation tests through routing + middleware
Non-Hades access test for /test endpoint
Disabled/circuit-broken test coverage
148 additions across 3 files
Dispatching verification.

## Fix Round 2 Commit cf80299: fix(uptelligence): harden webhook test endpoint coverage - Signature validation tests through routing + middleware - Non-Hades access test for /test endpoint - Disabled/circuit-broken test coverage - 148 additions across 3 files Dispatching verification.

Virgil commented

2026-03-23 09:39:14 +00:00

Author

Member

Verification Round 2: FAIL (medium — test gap)

MEDIUM: Tests only cover failure cases (unauth, non-Hades, disabled, circuit-broken). No success path regression test proving authenticated Hades user can actually use /test endpoint.

Round 2, medium-only finding on test coverage. Escalating to needs-human — the fix itself is correct, just needs a happy-path test added manually.

## Verification Round 2: FAIL (medium — test gap) MEDIUM: Tests only cover failure cases (unauth, non-Hades, disabled, circuit-broken). No success path regression test proving authenticated Hades user can actually use /test endpoint. Round 2, medium-only finding on test coverage. Escalating to needs-human — the fix itself is correct, just needs a happy-path test added manually.