Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/Controller/HealthController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
/**
* Controller for health check endpoints.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-4
*
* @psalm-suppress UnusedClass
*/
class HealthController extends Controller
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: added class-level @SPEC tag] Rule: ADR-003 — Class docblock must carry @SPEC traceability tag linking to openspec tasks. Sniff: SpecTagSniff WARNING on HealthController class at line 38.

Expand All @@ -57,6 +59,8 @@ public function __construct(
/**
* Health check endpoint.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-4
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access; also requires SEC-01/@publicpage fix first] Rule: CWE-209 / OWASP A05:2021 — Three exception messages returned verbatim in health response (lines 123, 147, 170). checkDatabase() at line 123 returns 'failed: '.$e->getMessage(), checkOpenRegister() at line 147 does the same, and checkFilesystem() at line 170 does the same AND is missing a logger call. Once @publicpage is applied, unauthenticated callers receive raw database connection-string fragments, table names, or file paths on error. Fix: replace each with a generic string ('failed: database unavailable', 'failed: app check failed', 'failed: filesystem unavailable') and add $this->logger->error() in checkFilesystem() catch. 5-line bounded change — apply in same cycle as SEC-01.

*
* @NoCSRFRequired
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access to root-owned repo files] Rule: OWASP A01:2021 — Missing @publicpage annotation on HealthController::index(). Without @publicpage, Nextcloud's auth middleware intercepts all unauthenticated requests (container health probes) with a 302 redirect to the login page, making the endpoint non-functional. Fix: add * @PublicPage on a line above * @NoCSRFRequired in the index() docblock. This is a mechanical 1-line addition — apply in next cycle.

*
* @return JSONResponse Health status
Expand Down Expand Up @@ -126,6 +130,8 @@ private function checkDatabase(): string
* OpenRegister is a hard dependency for Procest. If it is not enabled,
* the overall health status MUST be "error".
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-4
*
* @return string 'ok' or error message
*/
private function checkOpenRegister(): string
Expand Down
14 changes: 14 additions & 0 deletions lib/Controller/MetricsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
/**
* Controller for exposing Prometheus metrics.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-1
* @spec openspec/changes/prometheus-metrics/tasks.md#task-2
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*
* @psalm-suppress UnusedClass
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: added class-level @SPEC tags + index() @SPEC + moved @SPEC after @param in getCached()] Rule: ADR-003 — class docblock missing @SPEC (SpecTagSniff WARNING); index() missing @SPEC (SpecTagSniff WARNING); getCached() had @SPEC before @param violating PEAR.Commenting.FunctionComment order (phpcs ERROR).

*/
class MetricsController extends Controller
Expand Down Expand Up @@ -67,6 +71,10 @@ public function __construct(
/**
* Return Prometheus metrics in text exposition format.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-1
* @spec openspec/changes/prometheus-metrics/tasks.md#task-2
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*
* @NoCSRFRequired
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[unfixed: no container write access to root-owned repo files] Rule: OWASP A01:2021 — Missing @publicpage annotation on MetricsController::index(). Without @publicpage, Nextcloud's auth middleware blocks all unauthenticated Prometheus scrape requests with a 302 redirect. The /api/metrics endpoint is non-functional for external scrapers. Fix: add * @PublicPage above * @NoCSRFRequired in the index() docblock. Mechanical 1-line addition — apply in next cycle.

*
* @return TextPlainResponse Prometheus-formatted metrics
Expand All @@ -83,6 +91,10 @@ public function index(): TextPlainResponse
/**
* Collect all metrics and format as Prometheus text.
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-1
* @spec openspec/changes/prometheus-metrics/tasks.md#task-2
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*
* @return string Prometheus exposition format text
*/
private function collectMetrics(): string
Expand Down Expand Up @@ -200,6 +212,8 @@ private function collectMetrics(): string
* @param callable $compute Callable that computes the value on cache miss
*
* @return mixed The cached or freshly computed value
*
* @spec openspec/changes/prometheus-metrics/tasks.md#task-3
*/
private function getCached(string $key, int $ttl, callable $compute): mixed
{
Expand Down
Loading