fix: prevent use of hs* symmetric keys as urls and disallow http urls by default #2076
Conversation
SkArchon
requested review from
Noroth,
StarpTech,
devsergiy and
jensneuse
as code owners
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change enforces explicit specification of allowed JWT algorithms in JWKS configurations, adds validation for insecure JWKS URLs, and removes HMAC-related authentication support. It updates function signatures, configuration schemas, and test cases to require algorithm lists and to optionally allow insecure JWKS URLs via a new boolean flag in configuration and function calls. Changes
Estimated code review effort3 (~40 minutes) Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
router/pkg/config/config_test.go (1)
1312-1329: Potential test failure due to new algorithm validation.The test at line 1321 uses
algorithms: [](empty array), which will now fail the minItems validation introduced earlier. This test should be updated to either:
- Remove the algorithms field entirely to test the missing property validation
- Use a valid non-empty algorithms array if testing other validation aspects
Apply this fix to maintain test correctness:
authentication: jwt: jwks: - - algorithms: [] + - url: "http://example.com"Or if you want to test the missing algorithms validation specifically:
authentication: jwt: jwks: - - algorithms: [] + - some_other_field: "value"
🧹 Nitpick comments (5)
router-tests/modules/set_scopes_test.go (1)
32-37: LGTM: Test updated for new function signature.The addition of
trueas the second parameter correctly updates the test for the modifiedNewJwksTokenDecoderfunction signature. Usingtruein tests is appropriate for allowing local/test JWKS URLs.Consider adding a test case that verifies the security behavior when
allowInsecureJwksUrlsisfalseto ensure the validation works as expected:func TestSecureJWKSValidation(t *testing.T) { _, err := authentication.NewJwksTokenDecoder(integration.NewContextWithCancel(t), zap.NewNop(), []authentication.JWKSConfig{ { URL: "http://insecure-example.com/.well-known/jwks.json", // HTTP URL RefreshInterval: time.Second * 5, }, }, false) // Disallow insecure URLs require.Error(t, err) // Should fail with insecure URL }router/pkg/config/config.schema.json (1)
1662-1665: Description contradicts newminItems: 1constraint
algorithmsnow requires at least one entry ("minItems": 1), yet the preceding description still claims “An empty list means that all algorithms are allowed.”
Either drop theminItemsconstraint or update the description to avoid misleading users/documentation.- "description": "The allowed algorithms for the keys that are retrieved from the JWKs. An empty list means that all algorithms are allowed.", + "description": "The allowed algorithms for the keys that are retrieved from the JWKs. At least one algorithm must be provided.",router-tests/utils.go (1)
75-75: Consider making the insecure URL allowance configurable for comprehensive testing.The hardcoded
truevalue forallowInsecureJwksUrlsis appropriate for test flexibility, but consider making this configurable per test to ensure some tests validate the secure behavior whenallowInsecureJwksUrlsisfalse.For example, you could add an optional parameter:
-func ConfigureAuthWithJwksConfig(t *testing.T, jwksConfig []authentication.JWKSConfig) []authentication.Authenticator { +func ConfigureAuthWithJwksConfig(t *testing.T, jwksConfig []authentication.JWKSConfig, allowInsecure ...bool) []authentication.Authenticator { + insecure := true + if len(allowInsecure) > 0 { + insecure = allowInsecure[0] + } - tokenDecoder, err := authentication.NewJwksTokenDecoder(NewContextWithCancel(t), zap.NewNop(), jwksConfig, true) + tokenDecoder, err := authentication.NewJwksTokenDecoder(NewContextWithCancel(t), zap.NewNop(), jwksConfig, insecure)router-tests/authentication_test.go (2)
978-1016: Good test coverage for insecure URL validation.The tests properly verify both rejection of HTTP URLs and acceptance of HTTPS URLs. However, consider improving the second test by using a mock server instead of a non-existent URL to avoid potential network timeouts.
2422-2432: Good backward compatibility handling.The function properly handles the case where no algorithms are specified by defaulting to BaseJwksAlgorithms. Consider updating the comment to be more specific about which algorithms are included in the base set.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
router-tests/authentication_test.go(24 hunks)router-tests/header_set_test.go(1 hunks)router-tests/modules/set_scopes_test.go(1 hunks)router-tests/ratelimit_test.go(1 hunks)router-tests/utils.go(2 hunks)router-tests/websocket_test.go(8 hunks)router/core/supervisor_instance.go(1 hunks)router/pkg/authentication/authentication.go(1 hunks)router/pkg/authentication/jwks_token_decoder.go(3 hunks)router/pkg/authentication/validation_store.go(3 hunks)router/pkg/config/config.go(1 hunks)router/pkg/config/config.schema.json(2 hunks)router/pkg/config/config_test.go(1 hunks)router/pkg/config/testdata/config_defaults.json(1 hunks)router/pkg/config/testdata/config_full.json(1 hunks)
🧠 Learnings (14)
📓 Common learnings
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/modules/set_scopes_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/config.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/authentication/validation_store.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
router/core/supervisor_instance.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/ratelimit_test.go (1)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
router/pkg/config/config_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/config.schema.json (4)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Learnt from: SkArchon
PR: #1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
Learnt from: endigma
PR: #2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
router-tests/authentication_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/testdata/config_full.json (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/testdata/config_defaults.json (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/utils.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/authentication/jwks_token_decoder.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/websocket_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
🧬 Code Graph Analysis (4)
router/core/supervisor_instance.go (1)
router/pkg/authentication/jwks_token_decoder.go (1)
NewJwksTokenDecoder(60-202)
router-tests/header_set_test.go (2)
router/pkg/authentication/jwks_token_decoder.go (2)
NewJwksTokenDecoder(60-202)JWKSConfig(43-53)router-tests/utils.go (1)
NewContextWithCancel(36-40)
router/pkg/authentication/jwks_token_decoder.go (1)
router/pkg/authentication/validation_store.go (1)
NewValidationStore(33-64)
router-tests/websocket_test.go (2)
router/pkg/authentication/jwks_token_decoder.go (2)
NewJwksTokenDecoder(60-202)JWKSConfig(43-53)router-tests/utils.go (1)
NewContextWithCancel(36-40)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: wundergraph/cosmo#2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/modules/set_scopes_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/config.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/authentication/validation_store.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
router/core/supervisor_instance.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/ratelimit_test.go (1)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
router/pkg/config/config_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/config.schema.json (4)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
Learnt from: SkArchon
PR: #1929
File: router/internal/circuit/manager.go:16-25
Timestamp: 2025-06-30T20:39:02.387Z
Learning: In the Cosmo router project, parameter validation for circuit breaker configuration is handled at the JSON schema level rather than through runtime validation methods on structs. The config.schema.json file contains comprehensive validation constraints for circuit breaker parameters.
Learnt from: endigma
PR: #2009
File: router/pkg/config/config.go:0-0
Timestamp: 2025-07-03T10:33:25.778Z
Learning: The CardinalityLimit field in the Metrics struct (router/pkg/config/config.go) is validated at the JSON schema level in config.schema.json with a minimum value constraint of 1, preventing zero or negative values without requiring runtime validation.
router-tests/authentication_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/testdata/config_full.json (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/config/testdata/config_defaults.json (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/utils.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router/pkg/authentication/jwks_token_decoder.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
router-tests/websocket_test.go (2)
Learnt from: SkArchon
PR: #2067
File: router/pkg/authentication/jwks_token_decoder.go:80-106
Timestamp: 2025-07-21T14:46:34.879Z
Learning: In the Cosmo router project, required field validation for JWKS configuration (Secret, Algorithm, KeyId) is handled at the JSON schema level in config.schema.json rather than through runtime validation in the Go code at router/pkg/authentication/jwks_token_decoder.go.
Learnt from: SkArchon
PR: #2067
File: router/pkg/config/config.schema.json:1637-1644
Timestamp: 2025-07-21T15:06:36.664Z
Learning: In the Cosmo router project, when extending JSON schema validation for security-sensitive fields like JWKS secrets, backwards compatibility is maintained by implementing warnings in the Go code rather than hard validation constraints in the schema. This allows existing configurations to continue working while alerting users to potential security issues.
🧬 Code Graph Analysis (4)
router/core/supervisor_instance.go (1)
router/pkg/authentication/jwks_token_decoder.go (1)
NewJwksTokenDecoder(60-202)
router-tests/header_set_test.go (2)
router/pkg/authentication/jwks_token_decoder.go (2)
NewJwksTokenDecoder(60-202)JWKSConfig(43-53)router-tests/utils.go (1)
NewContextWithCancel(36-40)
router/pkg/authentication/jwks_token_decoder.go (1)
router/pkg/authentication/validation_store.go (1)
NewValidationStore(33-64)
router-tests/websocket_test.go (2)
router/pkg/authentication/jwks_token_decoder.go (2)
NewJwksTokenDecoder(60-202)JWKSConfig(43-53)router-tests/utils.go (1)
NewContextWithCancel(36-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build-router
- GitHub Check: integration_test (./events)
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: image_scan
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: build_test
🔇 Additional comments (31)
router/pkg/authentication/authentication.go (1)
82-82: LGTM: Clear error variable for algorithm validation.The new error variable follows Go naming conventions and provides a clear message for when algorithms are not specified in JWKS configuration.
router/pkg/config/config.go (1)
486-486: LGTM: Well-implemented security control.The new
AllowInsecureJwksUrlsfield correctly implements a security-by-default approach:
- Clear, descriptive field name
- Defaults to
falsefor security- Proper YAML and environment variable tags
- Follows existing naming conventions
router/pkg/config/testdata/config_defaults.json (1)
237-238: LGTM: Consistent test configuration update.The addition of
"AllowInsecureJwksUrls": falsecorrectly reflects the new configuration field with the expected default value and proper JSON formatting.router/pkg/config/testdata/config_full.json (2)
513-514: LGTM: Consistent configuration update.The addition of
"AllowInsecureJwksUrls": falseis consistent with the configuration structure changes and maintains the secure default.
487-495: JSON Schema permits null/empty JWKS algorithms
The JSON schema does not define anyrequiredorminItemsconstraint on theAlgorithmsorAlgorithmproperties, sonullor empty values are valid by design. Runtime warnings injwks_token_decoder.gohandle missing algorithm values for backwards-compatibility rather than enforcing a hard validation rule.
- No schema-level enforcement of non-empty
Algorithms/Algorithmwas found inrouter/pkg/config/config.schema.json.- Runtime logic issues only warnings when an algorithm is absent.
- The test entry in
router/pkg/config/testdata/config_full.json(lines 487–495) remains consistent with these rules.No changes required.
Likely an incorrect or invalid review comment.
router-tests/header_set_test.go (1)
268-268: LGTM! Test correctly updated for new function signature.The addition of
trueas theallowInsecureJwksUrlsparameter is appropriate for test environments where insecure URLs may be used for simplicity. This aligns with the broader security enhancement to prevent insecure JWKS URLs in production while maintaining test flexibility.router-tests/ratelimit_test.go (1)
264-264: LGTM! Consistent test update for enhanced security.The addition of
truefor theallowInsecureJwksUrlsparameter maintains test functionality while adapting to the enhanced JWKS URL security validation. This is consistent with other test file updates in the PR.router/core/supervisor_instance.go (1)
267-267: LGTM! Proper integration of security configuration.The change correctly passes the
jwtConf.AllowInsecureJwksUrlsconfiguration value to the token decoder, enabling administrators to control JWKS URL security policy. This properly implements the security enhancement while maintaining configurability.router-tests/websocket_test.go (1)
129-129: LGTM: Function calls correctly updated for new signatureAll calls to
authentication.NewJwksTokenDecoderhave been properly updated to include the newallowInsecureJwksUrlsparameter withtrueas the value. This is appropriate for test scenarios where:
- Test environments often use insecure (HTTP) endpoints
- Security restrictions that apply in production should not block test execution
- The tests need to verify functionality regardless of URL scheme
The consistent pattern across all test cases ensures uniform behavior and aligns with the PR's objective of making HTTPS the default while providing an override mechanism.
Also applies to: 178-178, 227-227, 286-286, 344-344, 406-406, 455-455, 856-856
router/pkg/authentication/validation_store.go (2)
20-31: Security improvement: HMAC algorithms correctly removedThe removal of HMAC algorithms (HS256, HS384, HS512) from
supportedAlgorithmsaligns with the PR objective of preventing the use of symmetric keys as URLs. This is a security best practice as:
- HMAC algorithms use symmetric keys that should not be exposed in JWKS URLs
- Asymmetric algorithms (RS*, PS*, ES*, EdDSA) are more appropriate for JWKS-based JWT verification
- This change enforces the use of public key cryptography for JWT validation
33-33: Well-implemented error handling for algorithm validationThe function signature change to return
(jwkset.Storage, error)and the enforcement of explicit algorithm specification is a good security practice. The implementation correctly:
- Returns
errNoAlgorithmsSpecifiedwhen no algorithms are provided, preventing the creation of a store with all supported algorithms- Maintains the error-aware signature consistently throughout the function
- Returns appropriate success/error states based on validation results
This change enforces explicit algorithm specification, which improves security by preventing overly permissive configurations.
Also applies to: 50-52, 63-63
router/pkg/config/config.schema.json (1)
1690-1694: Potentially breaking change – existing configs withoutalgorithmswill now failRequiring both
"url"and"algorithms"("required": ["url", "algorithms"]) hard-fails any previously valid configuration that only supplied aurl.
Past learnings emphasised backward compatibility by preferring warnings over hard validation (see JWKS schema decisions in earlier PR #2067). Please double-check that:
- All production configs already include
algorithms, or- A migration path / version gate is in place, or
- Runtime code gracefully handles validation failures and surfaces clear error messages.
If this risk hasn’t been evaluated, consider soft-deprecating the missing field first (e.g. via schema
warningpattern or runtime warning) before enforcing it.router-tests/utils.go (2)
22-34: Excellent security improvement by excluding HMAC algorithms.The
BaseJwksAlgorithmslist correctly includes only asymmetric algorithms (RSA, ECDSA, RSA-PSS, EdDSA) and excludes HMAC algorithms, which aligns with the PR objective to prevent use of symmetric keys in JWKS configurations.
58-72: Good practice to use secure algorithms in test configurations.The update to set
AllowedAlgorithms: BaseJwksAlgorithmsensures that tests use the same secure algorithm list that would be used in production, providing better test coverage for the security restrictions.router/pkg/authentication/jwks_token_decoder.go (4)
8-8: Necessary import for URL validation functionality.The
net/urlimport is correctly added to support the new JWKS URL scheme validation.
60-60: Well-designed function signature with clear security parameter.The addition of
allowInsecureJwksUrls boolparameter provides explicit control over URL security validation with a descriptive name that makes the security implications clear.
73-81: Excellent security validation implementation.The URL validation logic is well-structured with proper error handling:
- Only enforces security when required (secure by default)
- Uses standard
url.ParseRequestURIfor robust URL validation- Specifically validates HTTPS scheme requirement
- Provides clear, informative error messages
This effectively implements the PR objective to disallow HTTP URLs by default.
83-98: Improved error handling for validation store creation.Moving the
NewValidationStorecall outside the struct initialization and adding explicit error handling is a good improvement. This ensures that algorithm validation errors are properly caught and reported, supporting the requirement for explicit algorithm specification.router/pkg/config/config_test.go (3)
1205-1221: Good test coverage for secure default behavior.The test correctly validates that:
- Algorithms must be explicitly specified in JWKS URL configurations
- The default
AllowInsecureJwksUrlsvalue isfalse(secure by default)- Valid configurations with required fields load successfully
1223-1238: Essential validation test for algorithm requirement.This test correctly validates that empty algorithms arrays are rejected with the appropriate error message, ensuring that JWKS configurations must explicitly specify allowed algorithms.
1240-1254: Comprehensive validation for missing algorithms field.This test ensures that the algorithms field is required for URL configurations, complementing the empty array test to provide complete validation coverage for algorithm specification requirements.
router-tests/authentication_test.go (10)
693-704: LGTM!The explicit specification of allowed algorithms enhances security by preventing algorithm confusion attacks.
804-812: LGTM!Consistent implementation of explicit algorithm specification across multiple JWKS configurations.
855-855: LGTM!Consistent addition of the boolean parameter for insecure URL allowance.
906-909: LGTM!Proper implementation of explicit algorithm specification.
1217-1217: LGTM!Proper error handling for the token decoder creation.
1235-1247: Good test coverage for algorithm validation.The test properly verifies that algorithms must be explicitly specified, aligning with the PR's security objectives.
1350-1351: LGTM!Proper error handling for OIDC discovery-based token decoder creation.
Also applies to: 1386-1387, 1404-1405
1463-1463: LGTM!Proper error handling maintained.
1594-1595: LGTM!Good use of the variadic parameter pattern for optional algorithm specification.
2009-2012: LGTM!Consistent implementation of explicit algorithm specification across all audience validation tests.
Also applies to: 2094-2097, 2184-2187, 2270-2273, 2353-2356, 2395-2397
SkArchon
changed the title
SkArchon
force-pushed
the
milinda/eng-7666-prevent-use-of-hs3256-hs-symmetric-keys-as-urls
branch
from
ddf1018 to
30916ac
Compare
SkArchon
force-pushed
the
milinda/eng-7659-wun-q225-09-lack-of-audience-aud-validation-in-jwt
branch
from
5d07aa2 to
28931b8
Compare
SkArchon
force-pushed
the
milinda/eng-7666-prevent-use-of-hs3256-hs-symmetric-keys-as-urls
branch
from
fa3b36e to
5a3befe
Compare
|
Discussed with @StarpTech, we will not do this change. |
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Warning
Breaking Changes
Checklist