Repository navigation
fix: resolve Aikido dependency, Go version and CI findings - #25
Conversation
bonifaido
left a comment
There was a problem hiding this comment.
Deps, GCP and CI changes look good. Two things before merging:
1. x/net and grpc are still vulnerable. govulncheck on this branch reports 6 reachable vulns; Aikido's target versions were stale:
golang.org/x/net0.55.0 → needs 0.60.0 (GO-2026-6603, 6610, 6611, 6612, 6617)google.golang.org/grpc1.82.1 → needs 1.83.1 (GO-2026-6348, reachable via SPIFFEFetchJWTSVID)
go get golang.org/x/net@v0.60.0 google.golang.org/grpc@v1.83.1 builds, tests pass, and govulncheck is clean afterwards (it also pulls google.golang.org/api to 0.264).
2. DecodeJWTClaims doesn't change the security posture. It's still unverified decoding, just no longer matched by the rule. riptideslabs/issues#697 was a false positive: all four call sites only read exp or telemetry claims from tokens fetched from the issuer over TLS, nothing authz-related. I'd revert to ParseUnverified and mark the finding as a false positive in Aikido. If we keep the helper, see the inline comment, and rename it to something like UnverifiedJWTClaims with a doc comment saying it never verifies, ideally under internal/. As an exported pkg/util func it's easy to misuse later, and Aikido won't catch that. I'd skip the bot's suggested fix, it just calls ParseUnverified again.
Good: CredentialsFromJSONWithType(..., google.ServiceAccount, ...) closes off external_account credential sources, the nil-exp check in OCI fixes a panic on main, and the action SHA is the upstream v7.0.1 tag (a # v7.0.1 comment next to it would help).
| ) | ||
|
|
||
| func DecodeJWTClaims(token string) (jwt.MapClaims, error) { | ||
| parts := strings.Split(token, ".") |
There was a problem hiding this comment.
strings.Split here reintroduces CVE-2025-30204 (fixed in golang-jwt v5.2.2): 1 MiB of dots allocates ~16 MiB, vs 384 B for ParseUnverified. strings.SplitN(token, ".", 4) keeps the len != 3 check working.
There was a problem hiding this comment.
Moot now: the helper was reverted in 3816545, so pkg/util/jwt.go is gone and the call sites are back on ParseUnverified.
|
Thanks, addressed:
|
bonifaido
left a comment
There was a problem hiding this comment.
LGTM, thanks. govulncheck is clean on the new head and tests pass.
riptideslabs/issues#697 is still open though. Can you mark it as a false positive in Aikido and close it? The four ParseUnverified sites only read exp or telemetry claims from tokens fetched from the issuer over TLS, so there are no authz decisions on unverified claims.
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
Signed-off-by: Peter Balogh <poke@riptides.io>
9b7c8c2 to
684859c
Compare
Resolves open Aikido findings for tokenex.
google.CredentialsFromJSONwithCredentialsFromJSONWithTypeexpclaim instead of panickingpersist-credentials: falseon checkoutFixes riptideslabs/issues#695
Fixes riptideslabs/issues#696
Fixes riptideslabs/issues#698
Fixes riptideslabs/issues#699