Support Local License Inspection Command #4812
Conversation
WalkthroughThis pull request introduces a comprehensive license inspection feature for the Bacalhau CLI. It adds a new Changes
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
cmd/cli/license/inspect.go (1)
31-65: Enhance error messages for better user experience.While the command setup and validation are thorough, consider improving the error messages to be more user-friendly and actionable:
- return fmt.Errorf("required flag \"license-file\" not set") + return fmt.Errorf("please specify the path to your license file using the --license-file flag") - return fmt.Errorf("file not found: %s", o.LicenseFile) + return fmt.Errorf("license file not found at path: %s\nPlease verify the file path and ensure you have read permissions", o.LicenseFile)test_integration/common_assets/licenses/test-license-invalid.json (2)
1-3: Add documentation for test fixturesSince this is a test fixture for invalid license scenarios, consider adding a comment explaining:
- What makes this license invalid
- Expected behavior when used
- How it differs from valid licenses
{ + // Test fixture for invalid license scenarios + // Expected behavior: License validation should fail due to <reason> "license": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjVuSm5GQ05TeUFUMVNRdnR6bDc4MllDZUdrV3FUQ3R2..." }🧰 Tools
🪛 Gitleaks (8.21.2)
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
2-2: Consider shorter expiry for test JWTsThe JWT token has an expiry date set to 2384881638 (year 2045). While this works for tests, consider:
- Using a shorter expiry period (e.g., 24 hours from generation)
- Adding test cases for expired licenses
🧰 Tools
🪛 Gitleaks (8.21.2)
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
test_integration/common_assets/licenses/test-license.json (1)
1-3: Enhance test coverage with additional license variationsThe current test fixture represents a basic valid license. Consider adding test fixtures for:
- Different license types (enterprise, premium, etc.)
- Various capability combinations
- Edge cases (empty metadata, missing optional fields)
Also, add documentation explaining the test fixture's purpose and expected validation behavior.
{ + // Test fixture for valid license validation + // License type: standard + // Capabilities: max_nodes=1 "license": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjVuSm5GQ05TeUFUMVNRdnR6bDc4MllDZUdrV3FUQ3R2..." }🧰 Tools
🪛 Gitleaks (8.21.2)
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cmd/cli/license/inspect.go(1 hunks)cmd/cli/license/inspect_test.go(1 hunks)cmd/cli/license/root.go(1 hunks)cmd/cli/root.go(2 hunks)cspell.yaml(1 hunks)pkg/lib/license/license_validator.go(2 hunks)pkg/lib/license/license_validator_test.go(3 hunks)test_integration/12_local_license_inspect_suite_test.go(1 hunks)test_integration/common_assets/licenses/test-license-invalid.json(1 hunks)test_integration/common_assets/licenses/test-license.json(1 hunks)test_integration/common_assets/nodes_configs/12_basic_orchestrator_config.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
pkg/lib/license/license_validator.go
[failure] 38-38:
The line is 356 characters long, which exceeds the maximum of 140 characters. (lll)
[failure] 45-45:
The line is 356 characters long, which exceeds the maximum of 140 characters. (lll)
🪛 GitHub Actions: Main Pipeline
pkg/lib/license/license_validator.go
[error] 38-38: The line is 356 characters long, which exceeds the maximum of 140 characters.
🪛 Gitleaks (8.21.2)
cmd/cli/license/inspect_test.go
21-21: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
23-23: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
25-25: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
27-27: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
pkg/lib/license/license_validator_test.go
114-114: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
115-115: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
116-116: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
117-117: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
121-121: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
113-113: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
122-122: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
123-123: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
124-124: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
125-125: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
126-126: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
127-127: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
128-128: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
129-129: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
test_integration/common_assets/licenses/test-license-invalid.json
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
test_integration/common_assets/licenses/test-license.json
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (13)
cspell.yaml (1)
36-36: LGTM! Appropriate addition to spell checker ignore paths.The new ignore path for license test files is well-placed and correctly formatted. This is a sensible addition since license files often contain long strings and identifiers that don't need spell checking.
cmd/cli/license/root.go (1)
8-19: LGTM! Well-structured command implementation.The command is properly structured with clear usage description and appropriate hook integration.
pkg/lib/license/license_validator.go (2)
98-101: Consider security implications of hardcoded JWKS keys.While offline validation is useful, hardcoding JWKS keys in the source code has security implications:
- Key rotation becomes a code change
- Keys are exposed in the source code
Consider implementing a more secure key management system or documenting the security trade-offs.
110-110: LGTM! Improved error messages.The error messages have been updated to use "license" instead of "token", providing better context for users.
Also applies to: 114-114
test_integration/12_local_license_inspect_suite_test.go (3)
41-57: LGTM! Comprehensive test coverage for basic license inspection.The test properly validates all essential license fields and error handling.
59-103: LGTM! Thorough JSON output validation.The test comprehensively validates the JSON output format, checking all fields including metadata.
105-115: LGTM! Invalid license handling is tested.The test properly verifies error handling for invalid licenses.
cmd/cli/root.go (1)
19-19: LGTM! Proper command registration.The license command is correctly imported and registered with the root command.
Also applies to: 108-108
cmd/cli/license/inspect.go (2)
1-29: LGTM! Clean imports and struct definitions.The imports are appropriate for the functionality, and the struct definitions follow Go conventions.
67-107: LGTM! Well-structured license info handling.The license info structs are well-defined, and the custom marshaling implementation correctly handles both JSON and YAML formats.
cmd/cli/license/inspect_test.go (1)
20-28: Review security implications of hardcoded test tokens.The test file contains hardcoded JWT tokens that might expose sensitive information. Consider:
- Using shorter, minimal test tokens with only required claims
- Adding a clear comment explaining these are test-only tokens
- Implementing token generation for tests instead of hardcoding
🧰 Tools
🪛 Gitleaks (8.21.2)
21-21: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
23-23: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
25-25: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
27-27: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
pkg/lib/license/license_validator_test.go (1)
429-482: LGTM! Comprehensive offline validator tests.The test cases thoroughly validate the offline license validator functionality.
test_integration/common_assets/licenses/test-license.json (1)
2-2: Verify license validation test coverageThe JWT payload contains several fields that should be validated:
- license_version: "v1"
- license_type: "standard"
- capabilities: {"max_nodes":"1"}
Please ensure the test suite includes validation for:
- Version compatibility checks
- License type restrictions
- Capability enforcement
Run this script to check test coverage:
✅ Verification successful
Test coverage for license validation is comprehensive ✅
The test suite in
test_integration/12_local_license_inspect_suite_test.goincludes thorough validation for all required fields:
- Version compatibility (
v1)- License type (via JWT validation)
- Capabilities (
max_nodes=1)- Additional validation for product name, expiry, and metadata
- Invalid license scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check test coverage for license validation scenarios # Look for test cases covering version validation echo "Checking version validation tests..." rg -l "license_version.*v1" test_integration/ # Look for license type validation echo "Checking license type validation..." rg -l "license_type.*standard" test_integration/ # Look for capability validation echo "Checking capability validation..." rg -l "max_nodes.*1" test_integration/Length of output: 436
Script:
#!/bin/bash # Check the content of the found test file echo "=== Content of test file ===" cat test_integration/12_local_license_inspect_suite_test.go echo -e "\n=== Broader search for license validation tests ===" # Search for any JWT or license validation related tests rg -l "func.*Test.*License" test_integration/ echo -e "\n=== Looking for JWT validation patterns ===" # Search for JWT validation code rg "jwt\." test_integration/ -A 5 echo -e "\n=== Looking for license validation patterns ===" # Search for license validation with more flexible patterns rg "license.*valid" -i test_integration/Length of output: 5234
🧰 Tools
🪛 Gitleaks (8.21.2)
2-2: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
test_integration/common_assets/nodes_configs/12_basic_orchestrator_config.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/lib/license/license_validator.go (2)
31-54: Consider adding key rotation documentation.The implementation of offline JWKS keys is secure, using RS256 algorithm and proper key identifiers. However, it would be beneficial to add documentation about:
- The process for rotating these hardcoded keys
- The relationship between these keys and the ones used in the online validation
Line range hint
127-128: Consider making time source configurable for testing.The time-based validation currently uses
time.Now()directly. Consider injecting the time source to:
- Make the validation more testable
- Handle edge cases where system time might be incorrect
Example approach:
type TimeSource interface { Now() time.Time } type RealTimeSource struct{} func (RealTimeSource) Now() time.Time { return time.Now() } // Update LicenseValidator to accept a TimeSource type LicenseValidator struct { keyFunc jwt.Keyfunc timeSource TimeSource }Also applies to: 132-133
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/lib/license/license_validator.go(2 hunks)
🔇 Additional comments (2)
pkg/lib/license/license_validator.go (2)
99-102: LGTM! Clean implementation of offline validator.The function follows good practices by reusing existing validation logic and maintaining consistency with other validator constructors.
111-111: LGTM! Improved error message clarity.Good improvement in error messages by using domain-specific terminology ("license" instead of "token").
Also applies to: 115-115
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
cmd/cli/license/inspect.go (4)
94-96: Handle unsupported output formats gracefullyThe current condition only checks for JSON and YAML formats. If an unsupported format is provided, it might lead to unexpected behavior. Consider adding an
elseclause to handle unsupported formats or set a default format.Apply this diff to improve the condition handling:
if o.OutputOpts.Format == output.JSONFormat || o.OutputOpts.Format == output.YAMLFormat { return output.OutputOne(cmd, nil, o.OutputOpts, claims) } + else if o.OutputOpts.Format != output.TextFormat { + return fmt.Errorf("unsupported output format: %s", o.OutputOpts.Format) + }
55-57: Simplify file existence checkYou can remove the explicit file existence check since
os.ReadFilewill return an error if the file does not exist. This simplifies the code by relying on error handling in one place.Apply this diff to simplify the code:
// Check if license file exists - if _, err := os.Stat(o.LicenseFile); os.IsNotExist(err) { - return fmt.Errorf("file not found: %s", o.LicenseFile) - }
70-73: Enhance error handling when reading the license fileConsider providing more detailed error messages for better troubleshooting, especially handling permission errors separately.
Apply this diff to improve error handling:
data, err := os.ReadFile(o.LicenseFile) if err != nil { - return fmt.Errorf("failed to read license file: %w", err) + if os.IsPermission(err) { + return fmt.Errorf("permission denied when reading license file %s: %w", o.LicenseFile, err) + } + if os.IsNotExist(err) { + return fmt.Errorf("license file not found: %s", o.LicenseFile) + } + return fmt.Errorf("unexpected error reading license file %s: %w", o.LicenseFile, err) }
77-79: Provide clearer error message when parsing license fileEnhancing the error message helps users understand the exact issue when the license file cannot be parsed.
Apply this diff to improve the error message:
if err := json.Unmarshal(data, &license); err != nil { - return fmt.Errorf("failed to parse license file: %w", err) + return fmt.Errorf("failed to parse license file %s as JSON: %w", o.LicenseFile, err) }test_integration/12_local_license_inspect_suite_test.go (1)
109-109: Avoid hardcoding exact error messages in testsAsserting on exact error messages can make tests brittle if the error messages change. Instead, consider checking for error types or matching substrings.
Apply this diff to generalize the error assertion:
s.Require().ErrorContains(err, "invalid license") - s.Require().ErrorContains(err, "invalid license: failed to parse license: token signature is invalid")Alternatively, if possible, assert on the error type or use more resilient error checking methods.
cmd/cli/license/inspect_test.go (1)
378-379: Close file resources properly to prevent resource leaksWhen writing to files in tests, ensure file handles are properly closed. Although
os.WriteFilehandles this internally, if you switch to usingos.Createandfile.Write, remember to deferfile.Close().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/cli/license/inspect.go(1 hunks)cmd/cli/license/inspect_test.go(1 hunks)test_integration/12_local_license_inspect_suite_test.go(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
cmd/cli/license/inspect_test.go
22-22: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
24-24: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
26-26: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
28-28: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (2)
test_integration/12_local_license_inspect_suite_test.go (1)
45-49: Check for unexpected errors in command executionEnsure that the command execution error is checked properly to handle unexpected failures in
TestValidateLocalLicense.cmd/cli/license/inspect_test.go (1)
22-28: Address security warnings from static analysis toolsStatic analysis tools have flagged these JWT tokens as potential secrets. Verify that these tokens are safe for inclusion in the codebase and do not expose any sensitive information.
Run the following script to search for hardcoded tokens and ensure they are safe:
✅ Verification successful
JWT tokens are safe test fixtures
These JWT tokens are specifically crafted test fixtures that:
- Only appear in test files
- Contain clearly marked test data ("bacalhau-integration-tests", "test-customer-id-123")
- Include intentionally invalid/expired tokens for negative testing
- Don't expose any sensitive production information
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for hardcoded JWT tokens in the codebase. # Command to find lines containing JWT tokens rg --lines-with-matches 'eyJ[a-zA-Z0-9_-]{10,}' --glob '*.go' # Expected Result: Only test files contain hardcoded tokens, and they are safe for inclusion.Length of output: 522
Script:
#!/bin/bash # Search for JWT tokens and show surrounding context rg -l 'eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+' --glob '*.go' # Decode the JWT tokens (header and payload parts only) echo "=== Decoding JWT tokens ===" echo "validOfficialTestLicense payload:" echo "eyJhbGciOiJSUzI1NiIsImtpZCI6IjVuSm5GQ05TeUFUMVNRdnR6bDc4MllDZUdrV3FUQ3R2MWZ5SFVRa3hyTlUiLCJ0eXAiOiJKV1QifQ.eyJwcm9kdWN0IjoiQmFjYWxoYXUiLCJsaWNlbnNlX3ZlcnNpb24iOiJ2MSIsImxpY2Vuc2VfdHlwZSI6InN0YW5kYXJkIiwibGljZW5zZV9pZCI6ImU2NmQxZjNhLWE4ZDgtNGQ1Ny04ZjE0LTAwNzIyODQ0YWZlMiIsImN1c3RvbWVyX25hbWUiOiJiYWNhbGhhdS1pbnRlZ3JhdGlvbi10ZXN0cyIsImN1c3RvbWVyX2lkIjoidGVzdC1jdXN0b21lci1pZC0xMjMiLCJjYXBhYmlsaXRpZXMiOnsibWF4X25vZGVzIjoiMSJ9LCJtZXRhZGF0YSI6e30sImlhdCI6MTczNjg4MTYzOCwiaXNzIjoiaHR0cHM6Ly9leHBhbnNvLmlvLyIsInN1YiI6InRlc3QtY3VzdG9tZXItaWQtMTIzIiwiZXhwIjoyMzg0ODgxNjM4LCJqdGkiOiJlNjZkMWYzYS1hOGQ4LTRkNTctOGYxNC0wMDcyMjg0NGFmZTIifQ" | base64 -d 2>/dev/null || echo "Invalid base64"Length of output: 973
🧰 Tools
🪛 Gitleaks (8.21.2)
22-22: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
24-24: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
26-26: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
28-28: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
Add
bacalhau license inspectCommandSummary
Added a new CLI command
bacalhau license inspectthat allows users to inspect and validate Bacalhau license files. The command supports both offline validation and multiple output formats (table, JSON, YAML).Features
bacalhau license inspectcommand with the following capabilities:Implementation Details
inspect.goimplementing the license inspection commandNewOfflineLicenseValidatorwith hardcoded JWKS verification keys for offline validationUsage Examples
Example output
Test Coverage
Summary by CodeRabbit
Release Notes
New Features
licensecommand to the CLI for inspecting and validating local license information.Testing
Improvements
Configuration