-
Notifications
You must be signed in to change notification settings - Fork 8
feat: [LC-1103] Dynamic Boost Templates #905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ffdb51a The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for staging-learncardapp canceled.
|
✅ Deploy Preview for learncarddocs canceled.
|
✅ Deploy Preview for learncarddocs canceled.
|
|
@claude please review this PR and provide a recommendation for mergin |
|
Claude finished @Custard7's task —— View job Code Review: Dynamic Boost Templates PR ✅ RECOMMEND MERGEReview Checklist:
✅ STRONG APPROVAL - MERGE READYThis is an excellent implementation of dynamic templating. The code quality, security, testing, and backwards compatibility are all exemplary. 🔒 Security: Excellent
🧪 Testing: Comprehensive
🔄 Backwards Compatibility: Perfect
📚 Documentation: Comprehensive
🛠 Code Quality: Excellent
🎯 API Design: Well Designed
Minor Observations (not blockers):
Recommendation: MERGE ✅This PR adds significant value with zero breaking changes and excellent engineering practices. The templating feature will enable powerful personalization capabilities while maintaining full backwards compatibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ PR Review
LGTM
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ PR Review
The PR implements Mustache templating for dynamic boost credentials with proper JSON escaping and comprehensive test coverage. However, there's a critical security concern with the lenient verification logic for templated boosts that only validates the credential type.
2 issues detected:
🔒 Security - Only credential type is verified for templated boosts, allowing arbitrary credential content to pass verification
Details: The verification logic for templated boosts only checks that credential types match, allowing credentials with arbitrary content to pass verification as long as they have the correct type. This creates a security vulnerability where an attacker could send a credential with completely different data than the template intended (different achievement details, criteria, etc.) and it would still be considered valid.
File:services/learn-card-network/brain-service/src/helpers/boost.helpers.ts🐞 Bug - No error handling for template rendering and JSON parsing that could fail with invalid templates or data 🛠️
Details: The template rendering and JSON parsing operations lack error handling. If renderTemplateJson produces invalid JSON or if the parsing fails for any reason, the error will propagate uncaught. This could result in poor user experience and unclear error messages when template substitution fails.
File:packages/plugins/learn-card-network/src/plugin.ts (925-927)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
Co-authored-by: gitstream-cm[bot] <111687743+gitstream-cm[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ PR Review
The PR implements Mustache templating for Boosts with comprehensive test coverage. Error handling was added to the sendBoost method (resolving issue_2), but similar protection is missing in the send() method. The security concern from issue_1 regarding lenient verification of templated boosts remains unaddressed.
1 issues detected:
🐞 Bug - Template substitution operations can fail but have no error handling, unlike the identical code in sendBoost method 🛠️
Details: Template rendering and JSON parsing operations in the send() method lack error handling, unlike the similar code in sendBoost() above. If renderTemplateJson produces invalid JSON or parsing fails, the error will propagate uncaught, leading to poor user experience and unclear error messages.
File:packages/plugins/learn-card-network/src/plugin.ts (1051-1056)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
Co-authored-by: gitstream-cm[bot] <111687743+gitstream-cm[bot]@users.noreply.github.com>
🎫 Dynamic Boost Templates (LC-1103)
Summary
Implements Mustache-style dynamic templating for Boost credentials, allowing personalized data to be injected at issuance time via a new
templateDataparameter.Problem
Previously, Boost credentials were static templates. To issue personalized credentials (e.g., with recipient names, grades, dates), developers had to either:
Solution
Add
templateDatasupport to send() and sendBoost() methods that renders Mustache variables ({{variableName}}) in Boost templates at issuance time.Changes
New Files
Modified Files
brain-service/package.jsonmustachedependencynetwork-plugin/package.jsonmustachedependencytemplateDatatoSendBoostInputValidatorsendBoostViaSigningAuthoritynetwork-plugin/src/types.tstemplateDataoption to sendBoostnetwork-plugin/src/plugin.tsdocs/.../boost-credentials.mdFeatures
{{variableName}}syntax in boost templatestemplateDatais optional; existing code works unchangedTemplate Behavior
templateDataprovided, no variablesTesting
Test categories:
Breaking Changes
None. This is a fully backwards-compatible addition.
Documentation
docs/core-concepts/credentials-and-data/boost-credentials.md#dynamic-templates-with-mustachedocs/tutorials/create-a-boost.md#bonus-dynamic-templates-with-mustache-variables✨ PR Description
Purpose: Add Mustache-based dynamic templating to Boost credentials enabling personalized variable substitution at issuance time for scalable, reusable credential templates.
Main changes:
templateDataparameter to SendBoostInput validator and LearnCardNetworkPluginMethods.sendBoost allowing key-value variable substitutionGenerated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how