SK-2909, SK-2908-Added-fix-for-Policy-Voilation - #434
Open
skyflow-himanshupal wants to merge 2 commits into
Open
skyflow-himanshupal wants to merge 2 commits into
skyflow-himanshupal wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the Endor Labs policy violations (SK-2909) in the v1 SDK. It upgrades
jjwtand pinsjackson-core/jackson-databindto 2.18.11 inpom.xmlandsamples/pom.xml.Why
com.fasterxml.jackson.core:jackson-databind@2.9.6(SSRF, unsafe deserialization, serialization gadgets). It came in transitively throughio.jsonwebtoken:jjwt@0.9.1.com.fasterxml.jackson.core:jackson-core@2.13.0(GHSA-h46c-h94j-95f3 StackOverflowError on deeply nested input, fixed in 2.15.0; GHSA-r7wm-3cxj-wff9 async parsermaxNumberLengthbypass, fixed in 2.18.8). It wasa direct dependency.
ObjectMapperdirectly but never declaredjackson-databind, so it silently used whatever versionjjwt0.9.1 pulled in.jjwt@0.9.1(a single all-in-one jar) andjjwt-impl/jjwt-jackson@0.11.2, which include copies of the sameio.jsonwebtoken.impl.*classes.samples/pom.xmldepends on the publishedskyflow-java:1.15.0, so it brings in the same vulnerable versions.Goal
com.skyflow:skyflow-java(pom.xml) andorg.example:skyflow-javasdk-sample(samples/pom.xml).pom.xml:jjwt0.9.1 → 0.12.6 (same asmain/v2). It brings injjwt-api,jjwt-implandjjwt-jackson0.12.6.jjwt-impl/jjwt-jackson0.11.2 entries.jackson-databind2.18.11 as a direct dependency and raisedjackson-core2.13.0 → 2.18.11.samples/pom.xml: addedjackson-databind/jackson-core2.18.11 entries, which replace the vulnerable versions coming in throughskyflow-java:1.15.0.Jwts.builder()...signWith(SignatureAlgorithm.RS256, key)calls are deprecated in jjwt 0.12 but still work the same way.skyflow-java1.x release (none has the fix yet; that can follow the next v1 release).Testing
mvn dependency:treefor both projects resolves only jackson 2.18.11. Nojackson-databind@2.9.6orjackson-core@2.13.0is left.mvn test: 211 tests run, 204 pass. The 7 failures (BearerTokenTest,TokenTest,SignedDataTokensTest) need a real./credentials.json, and they fail the same way on the unchangedv1branch.Token.getSignedUserTokenwas called with a generated RSA-2048 key. The token has the same header (RS256) and claims (iss,key,aud,sub,exp). It verifies with the public key via jjwt 0.12.6, andTokenUtils.decoded()still readsexp.maven.compiler.target7. Anyone running the v1 SDK on Java 7 would break.main(v2) already requires Java 8, and v1reaches end of life on 2026-10-31.
Tech debt
jackson-databinddirectly instead of relying on it arriving transitively.signWith(SignatureAlgorithm, Key)/setExpirationAPI, which jjwt 0.12 deprecates. The sample's jackson pins can be removed once it moves to a fixedskyflow-javarelease.