Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,49 @@ class GraphqlHttpTransportTest {
.expectStatus().isForbidden
}

/**
* Verifies that a trusted browser origin with an access cookie but no CSRF proof is rejected
* before either downstream gateway can execute the mutation.
*/
@Test
fun `cookie-authenticated graphql mutation requires csrf proof before resolver`() {
Comment on lines +239 to +240

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Document the new public transport test with KDoc

The new public test method lacks the structured KDoc required by AGENTS.md for all public methods. Add a /** ... */ comment documenting the trusted-origin, access-cookie, and missing-CSRF-proof scenario and the expected rejection before downstream execution; the descriptive test name does not satisfy the explicitly required documentation format.

AGENTS.md reference: AGENTS.md:L202-L205

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@chatgpt-codex-connector Added structured KDoc to the cookie-authenticated GraphQL transport tests in 2c3ad703. The focused BFF transport test passed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

client.post().uri(ApiEndpoints.Bff.GRAPHQL)
.header("Origin", "https://app.example.test")
.cookie("squarewise_access", "access-jwt")
.bodyValue(mapOf("query" to "mutation { __typename }"))
.exchange()
.expectStatus().isForbidden

verifyNoInteractions(accountsGateway, expenseCoreGateway)
}

/**
* Verifies that matching CSRF proof allows a cookie-authenticated mutation to execute
* through the HTTP transport and return GraphQL data without errors.
*/
@Test
fun `cookie-authenticated graphql mutation executes with matching csrf proof`() {
val group = BffGroup("group-csrf", "CSRF", "TRIP", revision = 1)
`when`(expenseCoreGateway.createGroup(BffCreateGroup("CSRF", "TRIP", "EUR"), null))
.thenReturn(Mono.just(group))

client.post().uri(ApiEndpoints.Bff.GRAPHQL)
.header("Origin", "https://app.example.test")
.header("X-CSRF-Token", "nonce")
.cookie("squarewise_access", "access-jwt")
.cookie("squarewise_csrf", "nonce")
.bodyValue(
mapOf(
"query" to "mutation { createGroup(input: { name: \"CSRF\", kind: TRIP, currency: \"EUR\" }) { id } }"
)
)
.exchange()
.expectStatus().isOk
.expectBody()
.jsonPath("$.errors").doesNotExist()
.jsonPath("$.data.createGroup.id").isEqualTo("group-csrf")
}

@Test
fun `graphql route answers preflight only for an exact configured origin`() {
client.options().uri(ApiEndpoints.Bff.GRAPHQL)
Expand Down
35 changes: 35 additions & 0 deletions docs/tasks/details/OPS-33.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,38 @@ but the Java/Kotlin lane failed while compiling `:app:accounts:compileKotlin`
with `java.lang.OutOfMemoryError: Java heap space` (job `122209107187`). The
workflow now gives the manual Gradle extraction build a 4 GiB heap through
`GRADLE_OPTS`; the replacement hosted run and fork-PR evidence remain pending.

### Initial alert triage increment

The CodeQL ReDoS finding in `tools/contracts/validate_public_surface.py` is
addressed by replacing ambiguous repeated whitespace with horizontal
whitespace plus mandatory newline boundaries. Focused regression coverage
protects multiline resolver discovery and non-matching long input.

Commit `71ed1a3a` implements this increment. The focused validator tests,
public-surface validation, `make python-typecheck`, and `git diff --check`
passed.

### BFF CSRF alert triage increment

Added an end-to-end WebFlux transport regression proving a cookie-authenticated
GraphQL mutation is rejected before any gateway or resolver interaction when
the CSRF proof is missing. The existing custom `BrowserCsrfWebFilter` remains
the intentional enforcement boundary while Spring's generic CSRF filter stays
disabled for bearer-native requests.

Commit `5dc30569` implements this increment. The focused BFF transport and
CSRF tests passed.

### Hosted review follow-up

The PR review identified one remaining CodeQL regex path and two missing HTTP
coverage boundaries. The resolver pattern now removes overlapping horizontal
whitespace quantifiers, the regression suite bounds a long-space non-match in a
subprocess, and the BFF transport suite covers both rejected and successful
cookie-authenticated mutations. The new public test methods include structured
KDoc.

Validation for this follow-up passed with the focused Python unittest, public
surface validator, `make python-typecheck`, focused BFF transport test, and
`git diff --check`.
4 changes: 4 additions & 0 deletions docs/tasks/progress.md
Original file line number Diff line number Diff line change
Expand Up @@ -1704,5 +1704,9 @@ until API and E2E journey tests exercise it through public interfaces.
| 2026-10-27 | DOC-29 | Recorded Increment 6 evidence for adding `docs/operations/README.md` to the active-reference inventory and requiring its exact `AGPL-3.0-only` marker. | Focused unittest, `make license-validate`, and `git diff --check` passed; hosted GitHub license detection and repository About metadata remain post-merge checks, while copyright ownership confirmation is required before merge. | `9224f2f` |
| 2026-10-27 | DOC-29 | Closed review follow-ups by scoping stale-MIT detection to first-party statements, covering `licensed under MIT`, shipping `LICENSE` in the fast runtime image, and recording the pre-merge ownership requirement. | Focused unittest, `make license-validate`, `make contracts`, `make python-typecheck`, `make workflow-validate`, Dockerfile syntax validation, and `git diff --check` passed. | `33b1cfd` |
| 2026-10-28 | DOC-29 | Extended stale-MIT detection to cover the first-party wording `licensed under the MIT License` and added the matching regression assertion from PR #58 review. | Focused license-validation unittest, `make license-validate`, `make python-typecheck`, and `git diff --check` passed. | `20981431` |
| 2026-10-28 | OPS-33 | Removed ambiguous repeated whitespace from the GraphQL resolver regex flagged by CodeQL and added focused multiline/non-match regression tests. | `uv run --frozen --no-build python -m unittest tools.contracts.test_validate_public_surface`, `uv run --frozen --no-build python tools/contracts/validate_public_surface.py`, `make python-typecheck`, and `git diff --check` passed. | `71ed1a3a` |
| 2026-10-28 | OPS-33 | Added a WebFlux transport regression proving cookie-authenticated GraphQL mutations are rejected before gateway/resolver execution without a CSRF proof; the custom BFF filter remains intentional for browser cookies while bearer-native requests stay unaffected. | `./gradlew :app:bff:test --tests 'com.subhrodip.squarewise.bff.graphql.GraphqlHttpTransportTest' --tests 'com.subhrodip.squarewise.bff.config.BrowserCsrfWebFilterTest' --no-daemon --rerun-tasks --console=plain` passed; `make workflow-validate` and `git diff --check` passed. | `5dc30569` |

| 2026-10-28 | OPS-33 | Closed hosted review follow-up by removing the remaining overlapping resolver whitespace quantifier, adding bounded long-space regression coverage, documenting the public BFF tests with KDoc, and proving a cookie-authenticated GraphQL mutation succeeds with matching CSRF proof. | `uv run --frozen --no-build python3 -m unittest tools.contracts.test_validate_public_surface` passed (3 tests); public-surface validation passed (45 REST, 9 GraphQL roots); `make python-typecheck` passed; focused BFF transport test passed; `git diff --check` passed. | `2c3ad703` |


41 changes: 41 additions & 0 deletions tools/contracts/test_validate_public_surface.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Regression tests for public-surface parsing boundaries."""

from __future__ import annotations

import subprocess
import sys
import unittest

from tools.contracts.validate_public_surface import GRAPHQL_RESOLVER


class PublicSurfacePatternTest(unittest.TestCase):
"""Protect GraphQL resolver discovery from ambiguous whitespace matching."""

def test_resolver_pattern_accepts_multiline_annotations(self) -> None:
"""Resolver annotations may be separated from functions by blank lines."""
source = "@QueryMapping\n\n fun groups"
self.assertEqual(GRAPHQL_RESOLVER.findall(source), [("Query", "groups")])

def test_resolver_pattern_rejects_non_resolver_text(self) -> None:
"""A long newline-only gap without a function must not match."""
source = "@QueryMapping\n" + ("\n" * 1024) + "not_a_function"
self.assertEqual(GRAPHQL_RESOLVER.findall(source), [])

def test_resolver_pattern_rejects_long_space_run_within_timeout(self) -> None:
"""A nonmatch with long horizontal whitespace must remain bounded."""
source = "@QueryMapping\n" + (" " * 32_000) + "not_a_function"
script = (
"import sys; "
"from tools.contracts.validate_public_surface import GRAPHQL_RESOLVER; "
"raise SystemExit(bool(GRAPHQL_RESOLVER.findall(sys.argv[1])))"
)
subprocess.run(
[sys.executable, "-c", script, source],
check=True,
timeout=2.0,
)


if __name__ == "__main__":
unittest.main()
3 changes: 2 additions & 1 deletion tools/contracts/validate_public_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
GRAPHQL_ROOTS = {"Query": "QueryMapping", "Mutation": "MutationMapping", "Subscription": "SubscriptionMapping"}
GRAPHQL_FIELD = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\s*(?:\([^)]*\))?\s*:")
GRAPHQL_RESOLVER = re.compile(
r"@(Query|Mutation|Subscription)Mapping(?:\([^)]*\))?\s*(?:\r?\n\s*)*fun\s+([A-Za-z_][A-Za-z0-9_]*)"
r"@(Query|Mutation|Subscription)Mapping(?:\([^)]*\))?"
r"(?:[ \t]*\r?\n)*[ \t]*fun[ \t]+([A-Za-z_][A-Za-z0-9_]*)"
)
BRUNO_REQUEST = re.compile(r"(?m)^(get|post|put|patch|delete)\s*\{\s*\n\s*url:\s*([^\n]+)")
MATRIX_OPERATION = re.compile(r"^\|[^|]+\|[^|]+\|[^|]+\|\s*`([^`]+)`\s*\|", re.MULTILINE)
Expand Down
Loading