Java: Add integration test for autobuild truststore merging

[redsun82]

Summary

Adds integration test for autobuild truststore merging fix.

Changes

Extended buildless-inherit-trust-store/test.py with a second test test_autobuild_merge_trust_store that:

• Starts an HTTPS server with a custom CA certificate
• Configures a custom truststore via JAVA_TOOL_OPTIONS
• Sets CODEQL_PROXY_CA_CERTIFICATE to the custom CA
• Verifies Maven can connect to the HTTPS server (proving the truststore was merged, not replaced)

Used @pytest.mark.ql_test(expected=".autobuild.expected") and check_diagnostics.expected_suffix to allow different expected files per test function in the same directory.

[Copilot]

Pull request overview

Adds/extends Java integration coverage around HTTPS truststore handling to validate truststore inheritance (buildless) and truststore merging behavior (autobuild) related to CODEQL_PROXY_CA_CERTIFICATE.

Changes:

• Refactors the existing test into test_buildless and adds test_autobuild_merge_trust_store, sharing an _https_server helper.
• Introduces per-test expected output suffixes so multiple test functions can coexist in the same directory.
• Adds new .expected baseline files for buildless vs autobuild modes (including diagnostics/buildless-fetches).

Reviewed changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
java/ql/integration-tests/java/buildless-inherit-trust-store/test.py	Splits the test into buildless + autobuild cases and adds shared HTTPS server helper; configures per-test expected suffixes.
java/ql/integration-tests/java/buildless-inherit-trust-store/test.buildless.expected	Expected query output for the buildless case.
java/ql/integration-tests/java/buildless-inherit-trust-store/test.autobuild.expected	Expected query output for the autobuild case (includes a diagnostic line).
java/ql/integration-tests/java/buildless-inherit-trust-store/diagnostics.buildless.expected	Expected exported diagnostics for the buildless run.
java/ql/integration-tests/java/buildless-inherit-trust-store/diagnostics.autobuild.expected	Expected exported diagnostics for the autobuild run (currently empty).
java/ql/integration-tests/java/buildless-inherit-trust-store/buildless-fetches.buildless.expected	Expected buildless fetch URL(s) for the buildless run.

Comments suppressed due to low confidence (1)

java/ql/integration-tests/java/buildless-inherit-trust-store/test.py:84

• This test reads cert.pem and passes it as CODEQL_PROXY_CA_CERTIFICATE, but cert.pem is also the certificate presented by the local HTTPS server (server.py loads ../cert.pem). If autobuild regressed back to replacing the truststore with one that contains only CODEQL_PROXY_CA_CERTIFICATE, Maven would still trust https://localhost:4443 and the test could still pass. To specifically validate “merge, don’t replace”, use a proxy CA certificate that is different from the server’s cert/CA (or add an additional HTTPS fetch that requires the JAVA_TOOL_OPTIONS truststore to remain in effect).

        # Read the certificate to use as CODEQL_PROXY_CA_CERTIFICATE
        with open(cwd / "cert.pem", "r") as f:
            proxy_ca_cert = f.read()

        # Set JAVA_TOOL_OPTIONS to use our custom truststore
        # This is the key setting that was being overridden before the fix
        java_tool_options = f"-Djavax.net.ssl.trustStore={certspath}"

        # Run autobuild with the truststore configured
        # Before the fix: autobuild would create a new truststore with ONLY the proxy CA,
        #                 losing the custom CA from JAVA_TOOL_OPTIONS, causing PKIX failures
        # After the fix:  autobuild merges the system truststore + proxy CA
        codeql.database.create(
            build_mode="autobuild",
            _env={
                "JAVA_TOOL_OPTIONS": java_tool_options,
                "CODEQL_PROXY_CA_CERTIFICATE": proxy_ca_cert,
            },

[Copilot]

_https_server takes a cwd parameter but never uses it (the server is started with cwd="repo" relative to the current working directory). Either drop the parameter, or use it to build the repo path (e.g., based on cwd / "repo") so the helper is self-contained and less dependent on ambient CWD.

[Copilot]

The docstring/commentary says autobuild merges the system truststore with CODEQL_PROXY_CA_CERTIFICATE, but this test is explicitly setting a custom truststore via JAVA_TOOL_OPTIONS. To avoid confusion (and to accurately describe what’s being exercised), update the wording to reflect that autobuild should preserve/merge the truststore configured by JAVA_TOOL_OPTIONS (rather than replacing it) when adding the proxy CA.

[mbg]

As already discussed elsewhere in-depth, I don't think this is the right place or approach to testing this. If we decide to implement trust store merging in the autobuilder, then there are better ways of testing this than to rely on abusing CODEQL_PROXY_CA_CERTIFICATE.

I'd suggest we close this.

[mbg]

That's not what this test tests. This test is about CODEQL_JAVA_EXTRACTOR_TRUST_STORE_PATH. Both maven_opts and CODEQL_JAVA_EXTRACTOR_TRUST_STORE_PATH are based on certspath. Both are used independently.

[redsun82]

I see, I get now the background of CODEQL_JAVA_EXTRACTOR_TRUST_STORE_PATH. I would have probably opted for another approach rather than an in-production variable override just for testing: setting up a deep copy of JAVA_HOME as the new JAVA_HOME for an integration test where we inject our test certificates should be able to test the fix we did internally in that area I think? An overlay FS would be even less costly, but probably unfeasible on all platforms.

[mbg]

The existing test is about exercising a particular fallback mechanism in the BMN extractor. Why is there now a test for an unrelated change in the autobuilder here?

[redsun82]

this was to reuse resources without duplicating them. If we keep this test (which we may or may not do), we could just rename the directory to something more generic regarding trust stores, and either keep the two tests in the same file or even have two separate test files (sharing the resource directory).

With the new testing framework we don't need to have a 1:1 correspondence between directories and tests, and we should use that to avoid duplicating test resources.

[mbg]

This test abuses CODEQL_PROXY_CA_CERTIFICATE to put the Maven autobuilder into a situation where (without trust store merging) it only has access to this one certificate, and therefore fails when Maven tries to perform any networking that requires it to validate other certificates.

[redsun82]

My point here was that users seem to expect that trust stores passed via java tool options or maven options should still work in a default setup setting when CODEQL_PROXY_CA_CERTIFICATE, and, I thought, independently of all the proxy setup (including the other env variables for the proxy setup). Under this lens

and therefore fails when Maven tries to perform any networking that requires it to validate other certificates

is exactly the behaviour one might want to avoid.

However, I understand this assumption might be wrong in light of how our proxy works.

[mbg]

This should not be here.

[mbg]

Only the new test_autobuild_merge_trust_store test is about this. The existing test is unrelated.

[mbg]

In the case of the first (existing) test, that's mostly true for particular versions of the JRE and build-mode: none.

It does not really test this in the second (new) test where it puts the autobuilder into a situation where it can only trust one certificate, unless trust store merging is supported.