graphql: Add Circular Type Reference Detection

[ricekot]

Overview

When a GraphQL schema is imported into ZAP, it is processed to see if any object types in it create cycles (e.g. Query -> (Organization -> Repository -> PullRequest -> Commit -> Organization)). An alert is raised for each unique cycle that is found.

A combination of Tarjan's algorithm and Johnson's algorithm is used to find unique cycles in the schema type graph. Note that reserved types are excluded from processing. Thanks to the book Black Hat GraphQL for the idea on which algorithms to use, and to this YouTube video that helped me figure out what to actually do here.

Note on enum options refactor: I've tweaked how Enum options like ArgsTypeOption, RequestMethodOption, etc. are being handled. Earlier, we had to write a custom renderer for each enum so that we could show the right display name for each enum member in the UI. Now, instead of using getName, we override toString which helps us skip the custom renderer. However, this also changed the return type for the view endpoints for these options, so I had to manually handle those endpoints to return the Enum#name() instead of the UI friendly name.

Related Issues

• graphql: Add GraphQL-specific scan rules zaproxy#7324

Checklist

• Update help
• Update changelog
• Run ./gradlew spotlessApply for code formatting
• Write tests
• Check code coverage
• Sign-off commits
• Squash commits
• Use a descriptive title

[Copilot]

Pull Request Overview

This PR adds cycle detection for GraphQL schemas by checking for circular type references, and configures the exhaustiveness and alert limit for cycle detection.

• Introduces new cycle detection parameters and enums in GraphQlParam.
• Updates unit tests, API endpoints, UI options, and documentation to support circular reference detection.
• Refactors GraphQlQueryMessageBuilder and Requestor to align with the new cycle detection features.

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlParam.java	Added new cycle detection configuration options and updated enum methods.
addOns/graphql/src/test/java/org/zaproxy/addon/graphql/GraphQlCycleDetectorUnitTest.java	Introduced unit tests for cycle detection functionality.
addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlQueryMessageBuilder.java	Refactored query message building; supports multiple request methods.
addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlParser.java	Updated to invoke cycle detection and generate queries based on the new settings.
addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlOptionsPanel.java	Updated options UI to include cycle detection configuration.
Others (API, Fingerprinter, ChangeLog, etc.)	Updated to reflect cycle detection changes and configuration in various components.

Files not reviewed (1)

• addOns/graphql/src/main/resources/org/zaproxy/addon/graphql/resources/Messages.properties: Language not supported

Comments suppressed due to low confidence (1)

addOns/graphql/src/main/java/org/zaproxy/addon/graphql/GraphQlQueryMessageBuilder.java:51

• When appending the variables parameter in buildGetQueryMessage, the code incorrectly uses a second '?' instead of '&'. Use '&variables=' to append the variables after the already existing query parameter to ensure proper URL formation.

String updatedEndpointUrl = endpointUrl + "?query=" + URLEncoder.encode(query, StandardCharsets.UTF_8.toString());

[kingthorin]

Is the low confidence copilot comment actually an issue? (I haven't looked at the code yet)

[psiinon]

Checkmarx One – Scan Summary & Details – ae1d808a-ebcf-4800-9725-23740b59f637

Great job! No new security vulnerabilities introduced in this pull request

[ricekot]

Is the low confidence copilot comment actually an issue?

Yes, it was. I wish we had noticed it before but pretty cool that this tool found it.

[kingthorin]

I haven't tested yet. This is a lot of code to absorb 😀

I'll try to pull it, built it, and test it in the next few days.

[kingthorin]

Sorry, I do plan to test this but life got in the way 😔 Going to be a bit longer.

[ricekot]

I think I've addressed all comments. Also rebased onto main.

[kingthorin]

Looks good overall

[kingthorin]

LGTM