[polacektomas]

Checklist:

• Have you added unit tests for your change?

[github-actions]

Mend Scan Results

Status: ✅ No findings detected

SCA scan output

Identified 0 dependencies

Detected 0 vulnerabilities (0 Critical, 0 High, 0 Medium, 0 Low)





No Policy violations were detected

Project 'dependency-analysis-pr' was updated, for more information, visit the Mend platform: https://ibmets.whitesourcesoftware.com/app/orgs/Enterprise%20Applications/applications/summary?project=4c9fbc37-091e-46e9-b875-c0aefb00d5ec
Or the Core UI: https://ibmets.whitesourcesoftware.com/Wss/WSS.html#!project;token=bb6fda22a3f04173960765f5e68200ea76dafe65e1a8494aaa5ac6ce27a54843

Mend AI scan succeeded.

Support Token: 2bc8107f59ba0406991ca9037571c3dce1792489582073

SAST scan output

*no findings*

Full logs and artifacts

[rnc]

Why is this being wrapped in a try/catch? AbstractRestApiListingTest extends AbstractRestReportsTest which extends AbstractRestApiTest which has a @TestSecurity. I don't think this is needed

[polacektomas]

The tests were failing now with the roles added. The claude explanation was this:

The @testsecurity annotation lives on AbstractRestApiTest (the grandparent class), which sets up a fake security context with the pnc-users / pnc-users-admin roles. That annotation applies to the test methods themselves, but @AfterEach lifecycle methods can run in a different security context depending on how Quarkus propagates the @testsecurity identity.

The problem was: after adding @RolesAllowed to the REST endpoints (like /listings/blacklist), the dropTables() cleanup in @AfterEach would call getAllArtifactsFromList(PATH_BLACK_LIST) — which hits the now-secured /listings/blacklist endpoint. If the @testsecurity context wasn't propagated into the @AfterEach phase, that GET would return a 401/403 instead of 200, causing processGetRequest to call fail() and throw an AssertionError. That blew up the cleanup, and since it was an @AfterEach, it would mark the test as failed even if the actual test method passed.

The try-catch swallows that AssertionError so that cleanup failures (from missing security context) don't mask the real test results. It's a pragmatic workaround for the Quarkus @testsecurity scope not reliably covering lifecycle methods.

[rnc]

Interesting! I got Bob to review that and it says

The actual behaviour
Claude's explanation is correct in its conclusion but wrong about the mechanism.

The @TestSecurity identity is not propagated into @AfterEach because of Quarkus propagation scope per se — it's because @TestSecurity on a class works by injecting a QuarkusSecurityTestExtension JUnit5 extension that wraps each @Test method. JUnit5 lifecycle methods (@BeforeEach, @AfterEach) run outside that per-test-method wrapper. The fake identity is therefore not active on the server during @AfterEach HTTP calls, and GET /listings/blacklist returns 401/403.

So the try/catch is a genuine, necessary workaround for a real Quarkus @TestSecurity limitation.

But the workaround has a problem
The try/catch swallows silently when cleanup fails — but the cleanup is load-bearing. If dropTables() fails, test data from one test leaks into the next, potentially causing false passes (a test that should find an empty list finds a pre-populated one and doesn't notice) or false failures (a test that inserts a unique artifact fails because the previous test's data is still there).

The System.err.println will appear in the console but won't fail the test or indicate which subsequent test was corrupted.

Better approaches
Option 1 — @TestSecurity directly on the concrete test classes (simplest fix)

Move @TestSecurity down from AbstractRestApiTest onto each concrete test class. This is more verbose but means each @QuarkusTest class carries its own annotation, and Quarkus applies it to the whole class including lifecycle methods. This is the documented approach for ensuring @AfterEach has the identity.

Option 2 — Use QuarkusSecurityTestExtension explicitly in @AfterEach

Quarkus provides io.quarkus.test.security.TestIdentityController (in some versions) to programmatically activate a test identity. Less clean.

Option 3 — Separate the cleanup from the REST call

Instead of calling the secured /listings/blacklist endpoint to find what to delete, inject the BlackListService bean directly and call it in @AfterEach via CDI — no HTTP call, no auth check, no problem. This is the most robust solution for a @QuarkusTest.

Option 4 — Accept the try/catch but log a warning that's hard to miss

If none of the above are feasible right now, at minimum replace System.err.println with a JUnit5 assumption failure or a @AfterAll that verifies the list is empty, so leaked test data causes an explicit test failure rather than silent corruption.

Summary for the PR
The try/catch is justified — the root cause (Quarkus @TestSecurity not covering @AfterEach when inherited) is real. But the current implementation silently swallows a potentially data-corrupting failure. The comment explains why it was added, which is good. The remaining concern is that if cleanup fails, subsequent tests may produce unreliable results with no visible signal. Option 1 (push @TestSecurity to the concrete classes) or Option 3 (CDI injection for cleanup) would fix the root cause rather than papering over it.

So I think option 1 sounds better than trying to swallow the exception and it also makes it clearer.

[polacektomas]

I don't believe a word Bob says 😆, but I put it into test and claude helped:

Option 1 wouldn't work because Bob's own explanation contradicts it. They correctly state:

▎ @testsecurity works by injecting a QuarkusSecurityTestExtension that wraps each @test method. Lifecycle methods run outside that per-test-method wrapper.

That wrapping is per-test-method regardless of where the annotation lives. Moving @testsecurity from the parent class to the concrete class doesn't change which methods get wrapped — lifecycle methods are still outside the wrapper.

We also have empirical proof: we tried putting @testsecurity directly on the dropTables() method itself, and it still returned 401. If Quarkus respected @testsecurity on lifecycle methods at all, the method-level annotation would have worked. It didn't.

I changed the test to perform db cleanup directly and it passes now.

[michalovjan]

version into pom properties

[michalovjan]

This endpoint is not supported nor used anymore. Keep it under pnc-users-admin-only.

[michalovjan]

Suggested change

	@RolesAllowed({ "pnc-users", "pnc-users-admin" })
	@RolesAllowed({ "pnc-users", "pnc-app-da-user", "pnc-users-admin" })

Move to LookupImpl

[michalovjan]

push to Impl

[michalovjan]

Suggested change

	@RolesAllowed({ "pnc-users", "pnc-users-admin" })
	@RolesAllowed({ "pnc-users", "pnc-app-da-user", "pnc-users-admin" })

should be moved to BlackListImpl.

[michalovjan]

This endpoint should be hidden with @RolesAllowed({ "pnc-app-da-user", "pnc-users-admin" }). no pnc-users.

[michalovjan]

pnc-users-admin only