fix(gateway): replace placeholder token handling with JWT resource server - #142
fix(gateway): replace placeholder token handling with JWT resource server#142seonghobae wants to merge 15 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Fresh exact-head RCA for current The failing assertion is RCA: source behavior and source-contract test agree; authoritative operator documentation drifted to weaker wording. Smallest safe remedy is documentation-only: state explicitly that JWT mode requires a non-blank |
Buyer-visible security gap
The gateway previously exposed a class named
JwtAuthenticationFilterwhose validator accepted only the literal example valuevalid_token. That was not a defensible production identity boundary. This draft replaces the placeholder with Spring Security's maintained reactive OAuth 2.0 Resource Server JWT path while preserving a standalone fail-closed mode that requires no invented issuer, key, client secret, or CI credential.This is an independent, path-disjoint security slice from protected
develop; it does not deepen the durable-job stack or modify the CDC acknowledgement PR.Exact current identity
develop;622e5e6c3d534f230c390f10e3832efadfc01825;fix/gateway-jwt-resource-server;c6efe5abde600e9147e050d6bc52659b1493973d;6265d30070553cff4014b75ae76b2e947d628be5;Old checks, reviews, approvals, and synthetic-merge evidence do not transfer to later heads.
RCA: placeholder authentication boundary
Immediate cause: the historical hand-written filter treated one hard-coded bearer string as valid and did not establish cryptographic JWT verification.
Technical root cause: the WebFlux gateway lacked an explicit reactive OAuth 2.0 Resource Server JWT
SecurityWebFilterChainbinding/etl/**and/cdc/**to supported Spring Security verification infrastructure.Systemic cause: earlier tests exercised the placeholder class directly instead of proving the registered WebFlux security chain's runtime behavior.
The implemented remediation uses Spring Security's reactive resource-server/Jose stack, removes the direct JJWT placeholder dependency and literal token acceptance, leaves only actuator health/info intentionally public, protects workload routes in
jwtmode, rejects unknown security modes at startup, and keepsdenyas the secure standalone default. Deployment-owned issuer/JWK/audience/algorithm configuration remains external configuration rather than a newly invented secret contract.docs/security/gateway-jwt.mdrecords the control, rollback, failure behavior, and APA 7 primary references.Follow-on RCA: deny mode returned an inactive Basic challenge
After the JWT boundary implementation, exact head
9dba8a63c41e7dd090c7dcb775ef67b863e7a335still had one source-local CI failure. CI run31281592066, including macOS job93163731340, reachedGatewaySecurityConfigurationRuntimeTestand failed onlydenyModeExposesOnlyHealthAndInfoRoutes:/etl/testreturned401 UnauthorizedwithWWW-Authenticate: Basicinstead of the intended403 Forbiddenservice-disable response.The immediate symptom was the status mismatch. The first hypothesis that Basic authentication had simply not been disabled was falsified by exact source inspection:
httpBasic, form login, logout, and CSRF were already explicitly disabled. The technical root cause was that deny mode had no explicit authentication entry point, so exception translation could still select a default challenge in the isolated WebFlux test context. That produced a misleading Basic challenge even though deny mode is not an authentication mode.Remediation options and feasibility
execute_now— configure deny mode with Spring Security's reactiveHttpStatusServerEntryPoint(HttpStatus.FORBIDDEN). Supported by the current reactive API, requires no credential or new permission, changes only deny-mode failure translation, and is directly covered by the already-failing runtime test.reject— change the test to accept401/Basic. This would codify an inactive authentication mechanism instead of the documented service-disable contract and would not address the root cause.reject— add an identity-provider URL/key merely to eliminate the challenge. Deny mode deliberately exists so standalone startup does not invent deployment trust material.Commit
acf2b12023a6a39fd9357779dbac641db5471b49applies the smallest safe production fix: an explicit forbidden authentication entry point only indenymode. No existing test was weakened. Commitc6efe5abde600e9147e050d6bc52659b1493973drecords the resulting gateway security behavior inCHANGELOG.md.Test-first and aggregate workflow evidence
The original source-security RED remains auditable at
3e1590853606650337b9be88527cf68fd3bdc1e5, which added the boundary contract before production implementation.For the follow-on runtime defect,
GatewaySecurityConfigurationRuntimeTestwas already RED on exact head9dba8a63c41e7dd090c7dcb775ef67b863e7a335. After the minimal deny-mode entry-point repair, exact headacf2b12023a6a39fd9357779dbac641db5471b49ran the same hosted matrix and the gateway runtime tests passed on macOS, Ubuntu, and Windows. Final source headc6efe5abde600e9147e050d6bc52659b1493973dthen caused the complete pull-request workflow set to run.Current aggregate results associated with that source head are green:
31282034979: aggregate success;31282034962: success;31282034977: success;31282034968: success;31282034956: aggregate success;The conditional self-hosted test job is skipped and is not counted as positive evidence. Aggregate success is not automatically literal-source evidence.
Exact-source evidence RCA
Fresh job-log inspection proves that the current protected workflow definitions predate #121's literal-head source controls.
CI source identity
CI macOS job
93164820122invokedactions/checkoutwithout an explicit source-head ref. GitHub fetched6265d30070553cff4014b75ae76b2e947d628be5asrefs/remotes/pull/142/merge, checked out that synthetic merge, and then ran the Maven reactor. Therefore CI31282034979is useful merge-preview evidence but is not accepted as literal-head source proof forc6efe5ab.... The Ubuntu and Windows aggregate success does not cure that source-identity contract.Security Scan source identity
Security Scan hard Trivy job
93164819936likewise invoked default checkout, fetched and checked out synthetic merge6265d30070553cff4014b75ae76b2e947d628be5, and then executedtrivy fs .. Its zero-finding result therefore describes the synthetic merge tree, not the literal PR source tree. Scorecard job93164819898uses the same default synthetic-merge source identity. OSV independently checks explicit base/head revisions, and dependency review ran successfully, but those controls do not substitute for the hard Trivy filesystem gate's literal-source requirement.Accordingly, green CI and Security Scan aggregates are retained as compatibility/security-preview evidence only. Literal-head CI and hard scanner acceptance remain non-passing until protected workflow controls explicitly bind execution to the unchanged PR head.
Remediation feasibility
read_only_dependency— integrate the central literal-head scanner repair and ci: schedule NVIDIA OpenCode maintenance agent #121's repository-local exact-source CI/SBOM controls through their own writer leases, then rerun this unchanged source head under those protected definitions. This addresses the source-identity root cause.reject— treat the synthetic merge SHA as equivalent to the source head. That weakens the explicit exact-source acceptance contract and can hide source/base identity drift.reject— copy central scanner controls into this feature branch merely to manufacture passing evidence. The scanner control plane is owned by the separately leasedContextualWisdomLab/.githubloop, and the repository-local CI controls belong to the earlier ci: schedule NVIDIA OpenCode maintenance agent #121 integration boundary.reject— manually reinterpret aggregate green as exact-head proof. A green conclusion cannot change the revision that was actually checked out and scanned.Approval state
There is no qualifying independent non-author formal
APPROVEDreview on exact current source headc6efe5abde600e9147e050d6bc52659b1493973d. No reviewer, team, App identity, credential, or approval is invented to satisfy that governance requirement. Approval is not the sole remaining merge gate because literal-source CI and hard Security Scan evidence are independently unavailable.The current primary autonomous OpenCode formal-review route is also a read-only central dependency: automated dispatch receipts for mightyETL report that the repository is absent from
OPENCODE_REPOSITORY_DISPATCH_TARGETS, while Noema requires a qualifying current-head primary OpenCode approval before its independent verdict can qualify. Do not self-approve or bypass that sequence.Merge boundary
Keep this PR Draft. Do not merge until the unchanged source head has accepted literal-head CI and hard security evidence after the protected control-plane integrations, every required repository/governance gate passes, zero unresolved valid review finding remains, and a qualifying independent non-author formal
APPROVEDreview is anchored to that exact head. Queued, pending, absent, cancelled, skipped-required, failed, predecessor-head, stale-base, status-only, author-only, or synthetic-merge-only evidence is not passing.