Skip to content

gate-9 unsourced-denial: the credential can be resolved one frame down, in a private helper of the same class #221

Description

@rubenvdlinde

Measured on scholiq development (fc1dffe), full-tree run of hydra-gates at main (756fe89). One gate-9 finding, and it is false.

lib/Controller/PaymentTransactionController.php:277 method=callback
rule=public-page-annotation-with-unsourced-denial — this method is
#[PublicPage] and returns 401/403, but nothing in it resolves a credential
from the request …

Call path

callback() is a PSP status webhook. Its first statement is:

#[PublicPage]
#[NoCSRFRequired]
public function callback(): JSONResponse
{
    if ($this->isAuthenticCallback() === false) {
        return new JSONResponse(data: ['error' => 'Not authorized'], statusCode: Http::STATUS_UNAUTHORIZED);
    }

and the credential is resolved one frame down, in a private helper of the same class:

private function isAuthenticCallback(): bool
{
    $expectedToken = $this->appConfig->getValueString(
        app: Application::APP_ID,
        key: self::OPENCONNECTOR_CALLBACK_TOKEN_KEY,
        default: ''
    );

    if ($expectedToken === '') {
        $this->logger->warning('… refusing every callback until one is set.');
        return false;                     // fails CLOSED when unconfigured
    }

    $authHeader = (string) $this->request->getHeader('Authorization');

    return hash_equals('Bearer '.$expectedToken, $authHeader);
}

That is a bearer token read from the request, compared in constant time, failing closed when no token is configured — textbook-correct for the exact shape _SELF_AUTH_RE's own comment block describes (openconnector PaymentsController::webhook, portaliq ContributionController::inbox, …).

Why the exemption misses it

scan_file() matches _SELF_AUTH_RE against _strip_comments(head + body) of the flagged method only. callback()'s own body contains none of the alternatives:

  • no hash_equals( / hash_hmac( — those are in the helper
  • no Bearer literal — in the helper
  • no ->getHeader( — in the helper
  • ->\s*\w*[Tt]oken\w*\s*\( does not match ->isAuthenticCallback()
  • the docblock does name scholiq.openconnector_callback_token, and comments are correctly stripped before the test — so the one place the credential is named in-frame is deliberately blanked

So the finding is produced by the helper being a helper. Extracting an auth check into a well-named private method is the refactor PHPMD and the fleet's own complexity work push toward, and it converts a clean method into a gate-9 finding.

Suggested fix

Before reporting public-page-annotation-with-unsourced-denial, resolve $this-><name>( calls made from the flagged method against private/protected methods declared in the same file, and run _SELF_AUTH_RE over their bodies too. One level of indirection covers this shape; the gate keeps failing for a method that denies on nothing at all, which is what it exists for.

Companion: #219 fixes gate-12's element extraction, found in the same scholiq run (18 of 18 findings false).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions