From f2652cc0f876b309a18ff10570f9a1e5166f3460 Mon Sep 17 00:00:00 2001 From: Wolfgang Kozian Date: Sat, 24 Oct 2026 15:10:22 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(h4):=20Jackson=203=20polish=20=E2=80=94?= =?UTF-8?q?=20defaulted=20asString=20in=20webhooks,=20mapper=20guidance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IncomingWebhookService: no-arg JsonNode.asString() throws on object/array nodes under Jackson 3, so a correctly signed but unusual payload gave 500. All call sites use asString(""); regression test with null/object/array fields (was 500, now 200) - StringListConverter: JsonMapper.builderWithJackson2Defaults() to match the app-wide use-jackson2-defaults - ai-guide: inject Spring's mapper / builderWithJackson2Defaults, warn about asString() on untrusted JSON - JsonWireFormatGuardIntegrationTest: pin enum name() read/write and ISO durations Co-Authored-By: Claude Opus 5.5 --- .../application/IncomingWebhookService.kt | 26 +++++++++---------- .../infrastructure/StringListConverter.kt | 6 ++--- .../JsonWireFormatGuardIntegrationTest.kt | 5 ++++ .../GitHubWebhookControllerTest.kt | 22 ++++++++++++++++ mkdocs/developer-guide/ai-guide.md | 8 ++++-- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/backend/src/main/kotlin/com/taskowolf/integrations/application/IncomingWebhookService.kt b/backend/src/main/kotlin/com/taskowolf/integrations/application/IncomingWebhookService.kt index 8f7f1af3..e807159d 100644 --- a/backend/src/main/kotlin/com/taskowolf/integrations/application/IncomingWebhookService.kt +++ b/backend/src/main/kotlin/com/taskowolf/integrations/application/IncomingWebhookService.kt @@ -70,18 +70,18 @@ class IncomingWebhookService( "push" -> { val commits = node.path("commits") for (commit in commits) { - val sha = commit.path("id").asString() - val message = commit.path("message").asString() - val url = commit.path("url").asString() + val sha = commit.path("id").asString("") + val message = commit.path("message").asString("") + val url = commit.path("url").asString("") linkKeys(issueKeyParser.parseKeys(message), projectId, projectKey, IntegrationProvider.GITHUB, RefType.COMMIT, sha, url, message.take(100)) } } "pull_request" -> { val pr = node.path("pull_request") - val number = node.path("number").asString() - val title = pr.path("title").asString() - val url = pr.path("html_url").asString() + val number = node.path("number").asString("") + val title = pr.path("title").asString("") + val url = pr.path("html_url").asString("") val texts = listOf(title, pr.path("body").asString("")) val keys = texts.flatMap { issueKeyParser.parseKeys(it) }.distinct() linkKeys(keys, projectId, projectKey, IntegrationProvider.GITHUB, RefType.PR, number, url, title) @@ -90,22 +90,22 @@ class IncomingWebhookService( } private fun processGitLabPayload(node: JsonNode, projectId: UUID, projectKey: String) { - when (node.path("object_kind").asString()) { + when (node.path("object_kind").asString("")) { "push" -> { val commits = node.path("commits") for (commit in commits) { - val sha = commit.path("id").asString() - val message = commit.path("message").asString() - val url = commit.path("url").asString() + val sha = commit.path("id").asString("") + val message = commit.path("message").asString("") + val url = commit.path("url").asString("") linkKeys(issueKeyParser.parseKeys(message), projectId, projectKey, IntegrationProvider.GITLAB, RefType.COMMIT, sha, url, message.take(100)) } } "merge_request" -> { val mr = node.path("object_attributes") - val iid = mr.path("iid").asString() - val title = mr.path("title").asString() - val url = mr.path("url").asString() + val iid = mr.path("iid").asString("") + val title = mr.path("title").asString("") + val url = mr.path("url").asString("") val keys = issueKeyParser.parseKeys(title) + issueKeyParser.parseKeys(mr.path("description").asString("")) linkKeys(keys.distinct(), projectId, projectKey, IntegrationProvider.GITLAB, RefType.PR, iid, url, title) } diff --git a/backend/src/main/kotlin/com/taskowolf/integrations/infrastructure/StringListConverter.kt b/backend/src/main/kotlin/com/taskowolf/integrations/infrastructure/StringListConverter.kt index c1a0ad4b..f31e7cfb 100644 --- a/backend/src/main/kotlin/com/taskowolf/integrations/infrastructure/StringListConverter.kt +++ b/backend/src/main/kotlin/com/taskowolf/integrations/infrastructure/StringListConverter.kt @@ -15,9 +15,9 @@ class StringListConverter : AttributeConverter, String> { if (dbData.isNullOrBlank()) emptyList() else mapper.readValue(dbData, typeRef) private companion object { - // JPA instantiates converters itself (no Spring injection), so the converter owns one - // immutable Jackson 3 mapper. A list of strings is unaffected by Jackson 3 default changes. - val mapper: JsonMapper = JsonMapper.builder().build() + // JPA instantiates converters itself (no Spring injection), so the converter owns one immutable mapper. + // Jackson-2 defaults match the app-wide `spring.jackson.use-jackson2-defaults: true`. + val mapper: JsonMapper = JsonMapper.builderWithJackson2Defaults().build() val typeRef = object : TypeReference>() {} } } diff --git a/backend/src/test/kotlin/com/taskowolf/core/JsonWireFormatGuardIntegrationTest.kt b/backend/src/test/kotlin/com/taskowolf/core/JsonWireFormatGuardIntegrationTest.kt index 4a253ac9..63bcd657 100644 --- a/backend/src/test/kotlin/com/taskowolf/core/JsonWireFormatGuardIntegrationTest.kt +++ b/backend/src/test/kotlin/com/taskowolf/core/JsonWireFormatGuardIntegrationTest.kt @@ -19,6 +19,7 @@ import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandl import tools.jackson.databind.DeserializationFeature import tools.jackson.databind.MapperFeature import tools.jackson.databind.cfg.DateTimeFeature +import tools.jackson.databind.cfg.EnumFeature /** * M3 guard: HTTP and STOMP JSON must be written by the Jackson 3 converters @@ -98,5 +99,9 @@ class JsonWireFormatGuardIntegrationTest : IntegrationTestBase() { assertFalse(objectMapper.isEnabled(DeserializationFeature.FAIL_ON_TRAILING_TOKENS), "trailing tokens must stay lenient") assertFalse(objectMapper.isEnabled(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES), "unknown properties must be ignored") assertFalse(objectMapper.isEnabled(DateTimeFeature.WRITE_DATES_AS_TIMESTAMPS), "dates must be ISO strings") + assertFalse(objectMapper.isEnabled(EnumFeature.WRITE_ENUMS_USING_TO_STRING), "enums must be written by name()") + assertFalse(objectMapper.isEnabled(EnumFeature.READ_ENUMS_USING_TO_STRING), "enums must be read by name()") + // Spring Boot 3's Jackson2ObjectMapperBuilder also disabled this, so durations were already ISO-8601 (PT1H) + assertFalse(objectMapper.isEnabled(DateTimeFeature.WRITE_DURATIONS_AS_TIMESTAMPS), "durations must be ISO-8601 strings") } } diff --git a/backend/src/test/kotlin/com/taskowolf/integrations/GitHubWebhookControllerTest.kt b/backend/src/test/kotlin/com/taskowolf/integrations/GitHubWebhookControllerTest.kt index 99b0b91e..021445a6 100644 --- a/backend/src/test/kotlin/com/taskowolf/integrations/GitHubWebhookControllerTest.kt +++ b/backend/src/test/kotlin/com/taskowolf/integrations/GitHubWebhookControllerTest.kt @@ -109,4 +109,26 @@ class GitHubWebhookControllerTest : IntegrationTestBase() { .andExpect(jsonPath("$.refs[0].refType").value("COMMIT")) } + // H4: Jackson 3's no-arg asString() throws on object/array nodes; a signed but odd payload must not 500. + @Test + fun `signed payloads with null, object or array fields are accepted`() { + val token = registerAndLogin("gh4@test.com") + createProject(token, "GH4") + val secret = setupGitHubIntegration(token, "GH4") + + val payloads = listOf( + """{"ref":"refs/heads/main","commits":[{"id":{"sha":"x"},"message":{"text":"GH4-1"},"url":["u"]}]}""", + """{"number":7,"pull_request":{"title":null,"body":{"x":1},"html_url":"https://example.com/pr/7"}}""", + """{"number":[1],"pull_request":{"title":{"t":"GH4-1"},"body":null,"html_url":null}}""" + ) + for (payload in payloads) { + mockMvc.perform( + post("/api/v1/integrations/github/GH4/webhook") + .contentType(MediaType.APPLICATION_JSON) + .content(payload) + .header("X-Hub-Signature-256", hmacSigner.sign(payload, secret)) + ).andExpect(status().isOk) + } + } + } diff --git a/mkdocs/developer-guide/ai-guide.md b/mkdocs/developer-guide/ai-guide.md index 491d9915..3c69de44 100644 --- a/mkdocs/developer-guide/ai-guide.md +++ b/mkdocs/developer-guide/ai-guide.md @@ -260,8 +260,12 @@ class LabelServiceTest { `SimpMessagingTemplate.convertAndSend(dest, mapOf(...))` ist mehrdeutig → Payload `as Any`. - **Jackson 3:** `tools.jackson.databind.ObjectMapper`/`JsonNode`, `tools.jackson.core.type.TypeReference`, `tools.jackson.module.kotlin.readValue`. **Annotationen bleiben** `com.fasterxml.jackson.annotation.*`. - `JsonNode.asText()` ist deprecated → `asString()`. Jackson-Exceptions sind unchecked (`JacksonException`). - `ObjectMapper` ist immutable → eigene Mapper nur über `JsonMapper.builder()...build()`. Konfiguration über + `JsonNode.asText()` ist deprecated → `asString()`. **Achtung:** `asString()` ohne Argument **wirft** bei + Object-/Array-Knoten (Jackson 2 `asText()` gab `""`) → bei fremdem/untrusted JSON immer `asString("")` (H4). + Jackson-Exceptions sind unchecked (`JacksonException`). + Den von Spring konfigurierten `ObjectMapper` **injizieren**. Wo das nicht geht (z. B. JPA-`AttributeConverter`), + `JsonMapper.builderWithJackson2Defaults().build()` — nicht `JsonMapper.builder()`, dessen rohe Jackson-3-Defaults + (alphabetische Reihenfolge, strenger) vom Rest der App abweichen. Konfiguration über `spring.jackson.*`; `use-jackson2-defaults: true` hält Property-Reihenfolge und lockeres null-/Trailing-Token- Handling (Guards: `JsonWireFormatGuardIntegrationTest`, `JsonCompatCharacterizationIntegrationTest`). Jackson 2 (`com.fasterxml.jackson.databind`) nicht mehr importieren; es ist nur noch für springdoc da (transitiv From 196019c70d48efa4c3c7050f9595fa36c9180e27 Mon Sep 17 00:00:00 2001 From: Wolfgang Kozian Date: Sat, 24 Oct 2026 15:11:02 +0200 Subject: [PATCH 2/2] =?UTF-8?q?docs:=20backlog=20=E2=80=94=20H4/H5=20PRs?= =?UTF-8?q?=20open=20(#178/#177),=20new=20H6=20(Postgres=20port=20publishe?= =?UTF-8?q?d)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- .../specs/2026-07-07-backlog-overview.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-07-07-backlog-overview.md b/docs/superpowers/specs/2026-07-07-backlog-overview.md index a5f80e42..4ddec11b 100644 --- a/docs/superpowers/specs/2026-07-07-backlog-overview.md +++ b/docs/superpowers/specs/2026-07-07-backlog-overview.md @@ -34,8 +34,9 @@ | B5 | SSO über die Oberfläche nicht nutzbar (Link, nginx-Proxy, Callback-Route) | Bug/Full-Stack | ✅ **GEMERGT** (PR #174, 2026-10-24, noch nicht released): Link, Proxy, Callback-Route + Refresh-Token, Fragment statt Query, `TW_BASE_URL`; E2E-Smoke grün | | B6 | Unbekannte Pfade liefern 500 statt 404 | Bug/Backend | ✅ **GEMERGT** (PR #170, 2026-10-24, noch nicht released): Spring-`ErrorResponse`-Status respektiert (404/405/400) | | B7 | Dev-Profil mit H2 startet nicht (`gen_random_uuid()` ab V11) | Bug/Dev-Setup | ✅ **GEMERGT** (PR #172, 2026-10-24, noch nicht released): Dev-Profil auf lokales Postgres, H2 nur noch Test-Scope | -| H4 | Jackson-3-Feinschliff: `asString()` im Webhook-Eingang, Mapper-Empfehlung im ai-guide | Hardening/Docs | ⏸️ Zurückgestellt — Deferred Minors aus dem M3-Final-Review | -| H5 | Auth-Rate-Limit vertraut client-gesetztem `X-Forwarded-For` (umgehbar) bzw. teilt sich hinter nginx einen Bucket | Security/Hardening | 🆕 Neu (2026-10-24, Nebenbefund B5) — noch nicht umgesetzt | +| H4 | Jackson-3-Feinschliff: `asString()` im Webhook-Eingang, Mapper-Empfehlung im ai-guide | Hardening/Docs | 🔀 **PR #178 offen** (2026-10-24): Webhook-`asString("")` (500 belegt), `builderWithJackson2Defaults`, ai-guide, Guard-Asserts | +| H5 | Auth-Rate-Limit vertraut client-gesetztem `X-Forwarded-For` (umgehbar) bzw. teilt sich hinter nginx einen Bucket | Security/Hardening | 🔀 **PR #177 offen** (2026-10-24): XFF ignoriert, `X-Real-IP` nur von privatem Peer, 429 keyed | +| H6 | Prod-Compose veröffentlicht Postgres auf `5432:5432` (alle Interfaces) | Security/Ops | 🆕 Neu (2026-10-24, Nebenbefund H5) — noch nicht umgesetzt | ## M1 — Dependency-/Base-Image-Wartung (Dependabot-Bündel) > ✅ **AUSGELIEFERT** (2026-07-18, Release **v1.0.14**). Reines Wartungsrelease, @@ -438,7 +439,8 @@ Lokal gegen Postgres entwickeln (`--spring.datasource.url=jdbc:postgresql://…` Postgres (Testcontainers/Docker) umstellen und H2 + `spring-boot-h2console` entfernen, oder H2-Alias-Funktion. ## H4 — Jackson-3-Feinschliff (Deferred Minors aus dem M3-Final-Review) -> Status: **⏸️ Zurückgestellt**. +> Status: **🔀 PR #178 offen (2026-10-24)**. Der 500 im Webhook-Eingang war real (`JsonNodeException`, per Test belegt). +> Durations: Boot 3 schrieb sie schon als ISO-8601 → Guard pinnt `WRITE_DURATIONS_AS_TIMESTAMPS=false`. - `IncomingWebhookService` (Zeilen ~73–108): `JsonNode.asString()` ohne Default **wirft** unter Jackson 3 bei Object-/Array-Knoten (Jackson 2 `asText()` gab `""`) und liefert `""` statt `"null"` für JSON-null. Ein korrekt @@ -451,7 +453,7 @@ Postgres (Testcontainers/Docker) umstellen und H2 + `spring-boot-h2console` entf `READ_ENUMS_USING_TO_STRING` und `DateTimeFeature.WRITE_DURATIONS_AS_TIMESTAMPS` erweitern. ## H5 — Auth-Rate-Limit: Client-IP aus ungeprüftem `X-Forwarded-For` -> Nebenbefund beim B5-Sichten (2026-10-24), Status: **🆕 Neu — noch nicht umgesetzt**. +> Nebenbefund beim B5-Sichten (2026-10-24), Status: **🔀 PR #177 offen (2026-10-24)** — nginx unverändert (setzt `X-Real-IP` bereits). `AuthRateLimitFilter` nimmt den **ersten** Wert von `X-Forwarded-For`, sonst `remoteAddr`. `docker/nginx.conf` setzt den Header nicht, d. h. (1) ein Angreifer setzt pro Request einen anderen `X-Forwarded-For` und umgeht das @@ -461,3 +463,12 @@ Fix-Skizze: nginx `proxy_set_header X-Real-IP $remote_addr` (schon da) bzw. `X-F (überschreiben statt anhängen) und im Filter nur dem vom eigenen Proxy gesetzten Header trauen (oder `server.forward-headers-strategy` + `RemoteIpValve` mit `internal-proxies`); Test mit gefälschtem Header. Nebenbei: die 429-Message ist hart englisch (nicht keyed). + +## H6 — Prod-Compose veröffentlicht Postgres auf allen Interfaces +> Nebenbefund beim H5-Sichten (2026-10-24), Status: **🆕 Neu — noch nicht umgesetzt**. + +`docker-compose.prod.yml` mappt den `db`-Service mit `ports: - "5432:5432"`. Docker bindet das an `0.0.0.0` und +umgeht dabei typischerweise Host-Firewalls (ufw/iptables-INPUT); die Datenbank ist damit aus dem Netz erreichbar, +geschützt nur durch `TW_DB_PASS`. Backend und nginx brauchen den Port nicht (Docker-Netz). +Fix-Skizze: `ports` beim `db`-Service entfernen oder auf `127.0.0.1:5432:5432` beschränken (für lokale +Admin-Tools); Hinweis in `getting-started.md`/Release-Notes, da Betreiber, die von außen zugreifen, das merken.