feat(KSM-641): Add secret storage capability with < operator#147
Draft
feat(KSM-641): Add secret storage capability with < operator#147
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR adds secret storage capability to the KSM GitHub Action using the < operator syntax. The implementation includes comprehensive safeguards to prevent record corruption, permission-aware error handling, and file upload support while maintaining full backward compatibility.
Key Changes:
- Add bidirectional secret management with
<operator for storing values to Keeper - Implement comprehensive safeguards including protected field validation and record integrity checks
- Add file upload functionality and enhanced error handling with clear messages
Reviewed Changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.ts | Core implementation of store operations with safeguards and error handling |
| src/safeguards.ts | New safeguards module with field validation and record integrity checks |
| test-integration.js | Integration test runner for local testing with real KSM configuration |
| tests/safeguards.test.ts | Comprehensive unit tests for safeguards functionality |
| tests/main.test.ts | Enhanced unit tests including store operation parsing and error handling |
| tests/integration.test.ts | Real KSM integration tests for end-to-end validation |
| tests/integration-field-restrictions.test.ts | Integration tests for field restriction enforcement |
| action.yml | Updated action definition with new inputs for store functionality |
| package.json | Version bump to 1.2.0 and new test scripts |
| README.md | Comprehensive documentation with examples and migration guide |
| CHANGELOG.md | Detailed changelog documenting new features and changes |
| .env.local.example | Template for local testing configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,191 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
The shebang should use /usr/bin/env node for better portability across different Node.js installations.
Comment on lines
+380
to
+393
| function isValidUrl(url: string): boolean { | ||
| try { | ||
| new URL(url) | ||
| return true | ||
| } catch { | ||
| // Check if it's a relative URL or protocol-less | ||
| return /^(\/|www\.|[a-zA-Z0-9-]+\.)/.test(url) | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The URL validation regex pattern ^(\/|www\.|[a-zA-Z0-9-]+\.) may be too permissive and could match unintended strings. Consider using a more specific pattern or documenting the intended behavior for edge cases.
| const fileUpload: KeeperFileUpload = { | ||
| name: fileName, | ||
| title: fileName, | ||
| type: 'application/octet-stream', // Could be enhanced with mime-type detection |
There was a problem hiding this comment.
The comment indicates missing functionality. Consider implementing MIME type detection or creating a TODO/issue to track this enhancement.
|
|
||
| describe('Protected Field Restrictions', () => { | ||
| it('should reject modification of fileRef field', async () => { | ||
| const mockLogger = createMockLogger(['A7Pu-DNINF8d14VD5NGETA/field/fileRef < test-value']) |
There was a problem hiding this comment.
Hard-coded record UIDs like 'A7Pu-DNINF8d14VD5NGETA' should be extracted to constants at the top of the file or use environment variables for better maintainability.
This major enhancement adds comprehensive secret storage functionality to the KSM Action, allowing users to write/update values in Keeper records using the new < operator syntax alongside the existing > operator for reading. ## New Features ### Secret Storage with < Operator - Store values to record fields: record/field/password < 'newpassword' - Store from environment variables: record/field/notes < env:MY_VAR - Store from files: record/field/notes < file:./notes.txt - Upload files to records: record/file/document.pdf < file:./local-file.pdf - Support for mixed read/write operations in single action ### Comprehensive Safety System - Field type validation and sanitization - Protection against modification of system fields (fileRef, passkey, etc.) - Record integrity checks with automatic backup/restore - Smart field creation for standard fields (notes, text, etc.) - Input validation with security safeguards ### Enhanced Error Handling - Detailed field-not-found errors with record type and available fields - Helpful suggestions for correct field paths - Reduced error message repetition - Context-aware error messages with record information ### Automatic Retry Mechanism - Out-of-sync record detection and retry with exponential backoff - Configurable retry limits and timing - Graceful handling of concurrent record modifications ### Performance Monitoring - File upload timing and progress tracking - Performance warnings for slow operations - Detailed logging for troubleshooting ## Testing & Quality - Comprehensive test suite with 54+ tests - Integration tests for real KSM operations - GitHub Actions workflow testing - Field restriction and safeguards testing - Retry mechanism testing - Docker-based build pipeline - Full TypeScript type safety ## Configuration - Configurable test record UID for GitHub Actions - Environment-based configuration examples - Comprehensive documentation and testing guides ## Backward Compatibility - All existing > operator functionality preserved - No breaking changes to existing workflows - Enhanced but compatible action.yml interface This implementation provides a robust, secure, and user-friendly way to manage Keeper secrets bidirectionally while maintaining the highest standards for data protection and operational reliability.
maksimu
force-pushed
the
KSM-641-add-secret-storage-with-arrow-operator
branch
from
e214f16 to
71cdb79
Compare
Replace potentially dangerous regex pattern that could cause catastrophic backtracking with a safer, more specific pattern that follows RFC 5322 guidelines while avoiding polynomial time complexity. - Old pattern: /^[^\s@]+@[^\s@]+\.[^\s@]+$/ (vulnerable to ReDoS) - New pattern: Uses character classes with quantified ranges to prevent backtracking - Maintains email validation functionality while eliminating security risk - All existing tests continue to pass This addresses the high-severity security alert from GitHub CodeQL.
Properly quote GitHub Actions output variable to prevent syntax errors when the output contains special characters that could break bash conditional expressions. - Use intermediate variable assignment to safely handle output values - Prevents 'syntax error in conditional expression' issues - Maintains validation functionality while improving robustness
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.
This major enhancement adds comprehensive secret storage functionality to the
KSM Action, allowing users to write/update values in Keeper records using the
new
<operator syntax alongside the existing>operator for reading.🎯 New Features
Secret Storage with < Operator
record/field/password < 'newpassword'record/field/notes < env:MY_VARrecord/field/notes < file:./notes.txtrecord/file/document.pdf < file:./local-file.pdfComprehensive Safety System
Enhanced Error Handling
Automatic Retry Mechanism
Performance Monitoring
🧪 Testing & Quality
⚙️ Configuration
🔄 Backward Compatibility
>operator functionality preserved📊 Impact
This implementation provides a robust, secure, and user-friendly way to manage
Keeper secrets bidirectionally while maintaining the highest standards for
data protection and operational reliability.