Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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,18 @@ class GraphqlHttpTransportTest {
.expectStatus().isForbidden
}

@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)
}

@Test
fun `graphql route answers preflight only for an exact configured origin`() {
client.options().uri(ApiEndpoints.Bff.GRAPHQL)
Expand Down
22 changes: 22 additions & 0 deletions docs/tasks/details/OPS-33.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,25 @@ 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.
2 changes: 2 additions & 0 deletions docs/tasks/progress.md
Original file line number Diff line number Diff line change
Expand Up @@ -1704,5 +1704,7 @@ 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` |


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

from __future__ import annotations

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), [])


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]*)*[ \t]*fun[ \t]+([A-Za-z_][A-Za-z0-9_]*)"
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
)
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