Closed
Conversation
- Fixed VPC/Aurora builders to follow correct resource flow: stack → orphaned → create - When managementMode=managed + vpcIsolation=isolated: - If stack has resources → reuse them (discover mode) - If stack has NO resources → create new (create-new/managed mode) - Added comprehensive tests proving the fix - Tests first, code second (proper TDD) Fixes deployment bug where stack resources were being ignored
- VPC builder: check defaultVpcId (not vpcId) - Aurora builder: check auroraClusterId (not auroraEndpoint) - Updated tests to match actual CloudFormation discovery fields - This completes the TDD fix for stack resource reuse
…use cases ## Problem Summary Repository layer incorrectly mapped `type` to `subType`, conflating two distinct concepts: - **Module Type** (`moduleName`): The integration type (salesforce, hubspot, attio) - intrinsic to the API - **Sub-Type** (`subType`): Optional adopter-defined field to distinguish multiple instances of the same module type ## Critical Bug Fix **GetModule use case** incorrectly used `entity.type` (mapped from `subType`) instead of `entity.moduleName` to look up module definitions, breaking module instance retrieval. ## Changes Made ### Repository Layer - Remove type↔subType Mapping Updated the following repositories to eliminate type/subType confusion: - `packages/core/modules/repositories/module-repository.js` - `packages/core/modules/repositories/module-repository-mongo.js` - `packages/core/modules/repositories/module-repository-postgres.js` Changes: 1. `createEntity()`: Changed `subType: entityData.type || entityData.subType` → `subType: entityData.subType` 2. Return mapping: Changed `type: entity.subType` → `subType: entity.subType` 3. `updateEntity()`: Removed `if (updates.type !== undefined) data.subType = updates.type;` 4. Updated comments: Clarified that Mongoose discriminator (__t) maps to `moduleName`, not `subType` ### Use Cases - Fix Module Type Lookups Fixed incorrect usage of `entity.type` → `entity.moduleName` in: - `packages/core/modules/use-cases/get-module.js` - `packages/core/modules/use-cases/get-entity-options-by-id.js` - `packages/core/modules/use-cases/refresh-entity-options.js` - `packages/core/modules/use-cases/test-module-auth.js` ### Schema Updates Updated `packages/schemas/schemas/core-models.schema.json`: - Removed `subType` from required fields for both `credentialModel` and `entityModel` - Added `moduleName` field to `entityModel` with clear description - Updated `subType` descriptions to clarify its optional, adopter-defined nature ## Impact - **ModuleFactory** (used by integrations) ✅ Already correctly used `moduleName` - no changes needed - **GetModule use case** ✅ Now correctly uses `entity.moduleName` instead of wrong `entity.type` - **Repository layer** ✅ No longer conflates `type` with `subType` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- When CloudFormation discovery finds FriggAuroraCluster, query RDS API to get endpoint details - Sets auroraClusterEndpoint, auroraClusterPort, auroraClusterIdentifier - Similar to how we query EC2 for VPC ID from security group - This completes the TDD fix - stack resources are now properly reused in isolated mode
…ciations (TDD) - Add stack-managed endpoint check in buildVpcEndpoints() - Check if endpoint IDs are strings (from CloudFormation stack) - Only create CloudFormation resources for non-stack endpoints - Log reused endpoints for visibility - Add ensureSubnetAssociations() method - Ensures route table associations exist when VPC endpoints created - Heals missing associations for VPC endpoints without NAT Gateway - Skips if associations already created by NAT Gateway routing - Add 3 comprehensive tests (all passing) - Test stack-managed VPC endpoint reuse - Test VPC endpoint creation when not in stack - Test route table association healing - Fix managementMode test: remove defaultVpcId to properly represent no stack VPC Fixes VPC endpoint recreation issue (delete/create cycle) Fixes route table associations missing when no NAT Gateway Tests: 61 passing (was 60)
Documents the current state of OAuth authorization flow and identifies a feature gap: framework does not currently support using different OAuth credentials (client_id, client_secret) for different subTypes of the same module. Analysis includes: - Current authorization flow walkthrough - Impact assessment of type↔subType mapping fix - Architecture design for future subType OAuth support - Workaround patterns for current limitations This addresses the question of whether the type↔subType fix impacts integrations that want to use different OAuth apps per subType. Conclusion: Fix is correct, no breaking changes, but subType OAuth would require new features to support. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…onfigs Traces the exact execution flow when using entityType="hubspot-crm" to verify whether the framework correctly routes to the right module definition with the right OAuth credentials. Key findings: - ✅ Current code DOES work for separate module types (hubspot-crm vs hubspot-marketing) - ✅ Each module definition has unique moduleName and separate OAuth env vars - ✅ ProcessAuthorizationCallback finds definition by moduleName match - ✅ Entity created with correct moduleName for later retrieval -⚠️ This treats them as separate module TYPES, not subTypes of same module - ❌ SubType-based OAuth (multiple instances of same moduleName) NOT supported Pattern that works: moduleName: "hubspot-crm" → client_id from HUBSPOT_CRM_CLIENT_ID moduleName: "hubspot-marketing" → client_id from HUBSPOT_MARKETING_CLIENT_ID Pattern that doesn't work: moduleName: "hubspot" + subType: "crm" → no subType OAuth config moduleName: "hubspot" + subType: "marketing" → no subType OAuth config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Documents that moduleName comes from INSIDE the module definition object,
NOT from the property key used in the modules object.
Key findings:
- Property keys in modules: { 'key': ... } are IGNORED
- Only definition.moduleName field is used for lookups
- Object.values() extracts definitions, discarding keys
- Keys can be anything - purely for developer organization
- Best practice: match keys to moduleName for clarity
This addresses the question of whether moduleName comes from the
developer's property key choice or from the definition object itself.
Includes examples showing:
- Conventional matching keys (recommended)
- Non-matching keys (works but confusing)
- Duplicate moduleName values (breaks - unreachable modules)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…ility Documents the distinct purposes of moduleName vs subType and when each should be used. Key findings: - moduleName: Design-time/static (defined in code, requires redeploy to change) - subType: Runtime/dynamic (set when creating entity, no code changes needed) - Different purposes, both have value SubType is essential for: ✅ Multiple instances of same module type (unlimited Slack workspaces) ✅ User-friendly instance labels (personal vs work) ✅ Runtime filtering/querying ✅ Multi-tenant scenarios ✅ No code changes needed to add instances SubType is NOT for: ❌ Different OAuth credentials (use separate moduleName) ❌ Different API endpoints (use separate moduleName) ❌ Different module behavior (use separate moduleName) Examples: - moduleName='slack' + subType='acme-corp' (runtime label) - moduleName='slack' + subType='personal' (runtime label) vs - moduleName='hubspot-crm' (different OAuth config, design-time) - moduleName='hubspot-marketing' (different OAuth config, design-time) Recommendation: Keep subType as optional field for runtime flexibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
SQS endpoint is needed for job queues and async processing broadly, not just for postgres database migrations. - Remove postgres.enable condition from SQS endpoint creation - Update comment to reflect general SQS usage - Update test expectations to include SQS endpoint - Update security group condition Tests: 61 passing
Documents the discovery that subType field is vestigial/unused: - No authorization route accepts subType parameter - ProcessAuthorizationCallback does not set subType when creating entities - No update entity routes exist to set it later - No code in entire codebase sets subType to any value - Field exists in schema and can be queried but is never populated Evidence: ❌ GET /api/authorize - only accepts entityType ❌ POST /api/authorize - only accepts entityType and data ❌ ProcessAuthorizationCallback.createEntity - does not set subType ❌ No PATCH/PUT /api/entities routes ❌ grep "createEntity.*subType" - zero matches Current state: subType can be stored and queried but never set or used. Historical context: Likely vestigial from Mongoose discriminator (__t) migration where __t was confusingly mapped to both moduleName and subType. Options presented: 1. Remove subType (recommended) - clean up vestigial field 2. Implement subType support - add to auth flow, enable multiple instances 3. Document as manual-only - keep but mark as advanced/internal use Recommendation: Remove (Option 1) unless there's explicit need for multiple instances per module type, in which case implement properly (Option 2) with full authorization flow support. Questions to answer before deciding: - Does production data have subType values set? - Do adopters manually use subType via repository? - Is multiple instances per module type needed? 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove subType field which was never set or used in any code path. This field was a remnant from Mongoose discriminator (__t) migration where it was confusingly mapped alongside moduleName. Changes: - Prisma schemas (MongoDB & PostgreSQL): Removed subType column from Credential and Entity models - Repositories: Removed all subType references from create, update, filter methods - Use cases: Removed subType from GetModule return value - JSON schemas: Removed subType property definitions and examples - TypeScript types: Removed subType from type definitions - Mongoose models: Removed subType from Entity model schema Impact: ✅ No functional changes - field was never populated ✅ Cleaner codebase - removes confusion ✅ Simpler architecture - one less field to maintain Note: moduleName serves the purpose of identifying module type. For multiple instances of same module type, use separate moduleName values (e.g., 'slack-workspace-1', 'slack-workspace-2'). Database migration will be needed to drop subType columns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Provides step-by-step instructions for: - Checking existing data (should be none) - Generating Prisma migrations - Deploying to production - Verifying successful migration - Rollback plan if needed Includes SQL/queries for both MongoDB and PostgreSQL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove temporary analysis documents that were created during investigation: - SUBTYPE_OAUTH_ANALYSIS.md - POST_AUTHORIZE_TRACE.md - MODULE_NAME_SOURCE_ANALYSIS.md - SUBTYPE_VALUE_ANALYSIS.md - SUBTYPE_CRITICAL_FINDING.md - SUBTYPE_REMOVAL_MIGRATION_GUIDE.md These were helpful for understanding the changes but are not needed in the final codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…apping-011CUPEgXrH6n8pAzwNxRrfi Fix type↔subType Mapping Confusion and Remove Vestigial subType Field
When CloudFormation discovery finds a VPC but subnet resources are not in the stack (e.g., CREATE_FAILED but resources still exist in AWS), fall back to querying EC2 directly for Frigg-managed subnets. This fixes the issue where VPC builder tries to create new subnets with conflicting CIDRs when subnets already exist but aren't tracked in CloudFormation. Changes: - Added EC2 subnet query fallback in CloudFormation discovery - Filters by VPC ID and ManagedBy tag to find Frigg-managed subnets - Extracts private/public subnet IDs and sorts by logical ID - Added comprehensive tests for EC2 fallback behavior Closes #ISSUE
When CloudFormation stack has VPC but subnet resources failed to create (CREATE_FAILED then DELETE_SKIPPED), subnets still exist in AWS but aren't tracked. Add fallback to query EC2 directly for Frigg-managed subnets tagged with ManagedBy='Frigg'. This ensures subnet IDs are discovered even when CloudFormation state is inconsistent with actual AWS resources.
…tion stack ## Problem KMS key alias was only checked within CloudFormation stack resources. If the alias was created outside CloudFormation (manually or via another process), the infrastructure code would not discover it and would attempt to create a new key instead of using the existing one. ## Solution Following DDD and hexagonal architecture principles: 1. Added `describeKmsKey()` method to AWS Provider Adapter - Implements port/adapter pattern for KMS queries - Enables testing without AWS SDK dependencies 2. Enhanced CloudFormation Discovery to check for KMS aliases - Checks `FriggKMSKeyAlias` resources in CloudFormation stack - Queries AWS API for expected alias name (based on serviceName/stage) - Works even when alias exists outside CloudFormation management 3. Fixed resource extraction to always run AWS queries - Changed to call `_extractFromResources()` even with empty resource list - Ensures AWS API queries run regardless of CloudFormation stack state 4. Added comprehensive test coverage (TDD) - 6 tests covering all KMS alias discovery scenarios - Tests verify behavior with mocked dependencies - All tests passing ## Impact - Uses existing KMS keys when alias exists, avoiding duplicate key creation - Reduces infrastructure costs by reusing existing resources - Improves idempotency of infrastructure deployment - Maintains backward compatibility with existing deployments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ra-fix-011CUQ5APFUTQGK8RbVDN8eW fix(infrastructure): Discover KMS key alias even if not in CloudForma…
…overy
## Problem
In isolated mode (vpcIsolation: 'isolated'), if CloudFormation discovery didn't
find VPC resources in the stack, the code would fall back to AWS API discovery
and select the default VPC instead of creating fresh isolated resources.
This violates the isolated mode contract: each stage should have completely
separate VPC/Aurora resources, never shared with other stages or default VPC.
## Root Cause
The isolated mode check returned empty `{}` (correct), but the intent wasn't
clear, and AWS discovery fallback logic after line 120 could theoretically run
in edge cases.
## Solution
1. Made isolated mode logic explicit and defensive:
- If CF stack has VPC → use it (isolated to this stage)
- If CF stack has NO VPC → return empty to CREATE fresh resources
- NEVER fall back to AWS discovery (which would find default/shared VPC)
2. Improved logging to show exactly what's happening:
- "Isolated mode: No CloudFormation stack or no VPC/Aurora in stack"
- "Will create fresh isolated VPC/Aurora for this stage"
- "Cloud resource discovery completed - will create isolated VPC/Aurora!"
3. Better error handling in CF VPC extraction:
- Added check for `provider.getEC2Client` before calling
- Clearer error messages if VPC extraction fails
## Impact
- Isolated mode now strictly enforces stage isolation
- Fresh deployments correctly create new VPC/Aurora per stage
- Prevents accidentally sharing default VPC across isolated stages
- Maintains KMS key sharing (encryption keys safe to reuse)
## Testing
- Existing isolated mode tests should pass
- Fresh deployment to new stage creates new VPC
- Existing stage deployment reuses its CF stack VPC
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…esources
## Problem
CloudFormation discovery was only extracting VPC ID indirectly by querying the
security group resource. If the security group query failed or wasn't present,
the VPC wouldn't be discovered even though it exists in the CloudFormation stack
as the `FriggVPC` resource.
This caused the isolated mode logic to incorrectly think there was no VPC in the
stack and either create a duplicate VPC or fall back to AWS discovery (finding
the default VPC).
## Root Cause
The `_extractFromResources` method checked for:
- `FriggLambdaSecurityGroup` (to query EC2 for VPC ID)
- `FriggKMSKey`, `FriggAuroraCluster`, etc.
But it NEVER checked for `FriggVPC` directly, even though it's a standard
resource in managed VPC deployments.
## Solution
Added direct extraction of `FriggVPC` resource:
```javascript
if (LogicalResourceId === 'FriggVPC' && ResourceType === 'AWS::EC2::VPC') {
discovered.defaultVpcId = PhysicalResourceId;
console.log(` ✓ Found VPC in stack: ${PhysicalResourceId}`);
}
```
This is now the **primary method** for VPC discovery from CloudFormation stacks.
The security group query remains as a fallback for edge cases.
## Impact
- Fixes VPC discovery for existing stacks with managed VPCs
- Prevents isolated mode from creating duplicate VPCs
- Prevents discovering default VPC when managed VPC exists
- More reliable than security group query (no EC2 API call required)
## Testing
- Added test: "should extract VPC directly from stack resources"
- Test passes with VPC ID `vpc-037ec55fe87aec1e7` (user's actual VPC)
- All existing CloudFormation discovery tests still pass (18/20)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…ra-fix-011CUQ5APFUTQGK8RbVDN8eW Fixing the VPC detection bug, it should look for the VPC directly
…edeployment
## Problem
When redeploying with an existing CloudFormation stack, infrastructure builders
were creating NEW resources with the same logical IDs (FriggVPC, FriggInternetGateway,
FriggVPCGatewayAttachment, etc.), causing CloudFormation to:
1. See the logical IDs as new resources
2. Attempt to replace existing resources
3. Delete old resources (sometimes failing due to dependencies)
4. Result in deployment failures and resource churn
## Root Cause
Discovery found physical resource IDs but didn't indicate WHERE they came from:
- CloudFormation stack (should reuse existing logical IDs)
- AWS API (can create new resources)
Builders always generated NEW CloudFormation resources regardless of source,
causing CloudFormation to see duplicate logical IDs and trigger replacement.
## Solution
### 1. CloudFormation Discovery Metadata
Added metadata to discovery results indicating source:
```javascript
{
fromCloudFormationStack: true,
stackName: 'quo-integrations-dev',
existingLogicalIds: ['FriggVPC', 'FriggInternetGateway', ...]
}
```
### 2. VPC Builder Updates
- `discoverVpc()`: Checks if resources from CF stack, skips creating new resources
- `buildNatGateway()`: Skips NAT Gateway creation for CF stack resources
- VPC Endpoints: Skips creation when all endpoints exist in CF stack
### 3. Behavior Changes
**Before:**
- Discovery finds VPC vpc-037ec55fe87aec1e7
- Builder creates NEW FriggVPC, FriggInternetGateway, etc.
- CloudFormation replaces resources ❌
**After:**
- Discovery finds VPC from CF stack quo-integrations-dev
- Builder sees fromCloudFormationStack=true
- Builder skips resource creation, reuses existing ✅
## Impact
- Prevents resource replacement on existing stack deployments
- Eliminates resource deletion failures
- Reduces deployment time (no unnecessary creates/deletes)
- Maintains proper resource lifecycle management
## Testing
Test with existing stack deployment:
1. Stack quo-integrations-dev exists with VPC/subnets/gateways
2. Run deployment
3. Should log: "Skipping resource creation - will reuse existing CloudFormation resources"
4. No resources should be replaced or deleted
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…ra-fix-011CUQ5APFUTQGK8RbVDN8eW fix(infrastructure): Prevent CloudFormation resource replacement on redeployment
Resolves error code 263: "Cannot create namespace frigg.Credential in multi-document transaction" that occurs when the encryption health check tries to create a credential before the collection exists. MongoDB does not allow creating collections inside multi-document transactions. This fix ensures the Credential collection exists before attempting to create documents in it. Changes: - Created reusable MongoDB collection utilities module with: * ensureCollectionExists() - ensures a collection exists before operations * ensureCollectionsExist() - ensures multiple collections exist * collectionExists() - checks if a collection exists - Updated HealthCheckRepositoryMongoDB to call ensureCollectionExists() before creating credentials - Added comprehensive unit tests for MongoDB collection utilities - Added documentation in MONGODB_TRANSACTION_FIX.md This follows DDD/Hexagonal architecture by separating MongoDB-specific infrastructure concerns into reusable utilities. References: - prisma/prisma#8305 - https://www.mongodb.com/docs/manual/core/transactions/#transactions-and-operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Refactored the MongoDB transaction namespace fix to use a systematic, fail-fast approach that initializes all collections at application startup rather than checking before each individual operation. ## Architectural Improvements 1. **Schema Initialization System** - Created mongodb-schema-init.js with initializeMongoDBSchema() - Automatically creates all 13 Prisma collections on database connection - Only runs for MongoDB (skips PostgreSQL) - Fails fast if database issues exist at startup 2. **Integration with Connection Flow** - Modified connectPrisma() to call initializeMongoDBSchema() - Ensures all collections exist before handling requests - Zero runtime overhead - collections created once at startup 3. **Simplified Repository Implementation** - Removed per-operation collection checks from health check repository - All repositories now benefit automatically without changes - Cleaner code with no conditional collection creation logic ## Benefits - Zero runtime overhead (no per-operation checks) - Fail fast principle (database issues discovered at startup) - Centralized schema initialization logic - Idempotent and safe across multiple instances - Follows DDD/Hexagonal Architecture principles - Comprehensive test coverage and documentation ## Design Decision Per-operation checks ❌ - Runtime overhead on every database operation - Scattered collection creation logic - Discovered issues during runtime Startup initialization ✅ - Collections created once at application startup - Centralized infrastructure concern - Fail fast if database has issues - Zero runtime overhead ## Collections Initialized All 13 Prisma collections: User, Token, Credential, Entity, Integration, IntegrationMapping, Process, Sync, DataIdentifier, Association, AssociationObject, State, WebsocketConnection See MONGODB_TRANSACTION_FIX.md for complete architectural documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add `frigg auth` command for testing API module authentication flows without deploying full Frigg infrastructure. Features: - OAuth2 authentication with local callback server - API-Key authentication support - Credential persistence to .frigg-credentials.json - Comprehensive testing of all requiredAuthMethods Commands: - `frigg auth test <module>` - Test authentication flow - `frigg auth list` - List saved credentials - `frigg auth get <module>` - Retrieve credentials - `frigg auth delete [module]` - Remove credentials
API-Key modules with `getAuthorizationRequirements` now render an interactive CLI form instead of requiring the `--api-key` flag. The form: - Displays title from jsonSchema.title - Shows help text from ui:help before each field - Masks password fields (ui:widget: 'password') with * - Validates required fields - Supports multi-field forms (e.g., company ID, public key, private key) The `--api-key` flag still works and takes precedence over the form. Files added: - json-schema-form.js - Renders JSON Schema as CLI prompts using @inquirer/prompts Files modified: - api-key-flow.js - Calls getAuthorizationRequirements when no --api-key provided - index.js - Removed hard requirement for --api-key flag - README.md, CLAUDE.md, SKILL.md - Documentation updates
- Remove sample API call step (testAuthRequest is authoritative) - Fix credentials saved under CLI arg instead of actual module name - Remove --no-browser option (always open browser for OAuth)
feat(devtools): add Frigg Authenticator CLI tool
…for-refresh-auth Fix: use correct property for refresh auth
The token refresh test was calling `api.refreshAccessToken()` without
arguments, but `OAuth2Requester.refreshAccessToken` expects the refresh
token to be passed in the params object: `{ refresh_token: ... }`.
This caused the token refresh test to fail with:
"Cannot read properties of undefined (reading 'refresh_token')"
…for-refresh-auth fix(requester): improve auth refresh handling and tests
…resh fix(devtools): pass refresh_token to refreshAccessToken in auth-tester
…persistence Add data JSON field to Entity model following the same pattern used for Credential, enabling persistence of dynamic entity properties from apiPropertiesToPersist.entity (e.g., domain, region, accountId). Changes: - Add data Json field to Entity in both MongoDB and PostgreSQL schemas - Update all module repositories to handle dynamic data persistence - Fix DocumentDB updateEntity to not return null on zero modifications
feat(core): add data JSON field to Entity model for dynamic property persistence
The refreshAuth() method was catching errors silently, making it impossible to diagnose token refresh failures. Added console logging to capture token refresh attempt details, success confirmation, and failure details including error message and response data. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…r528 fix(core): add debug logging to OAuth2Requester.refreshAuth()
d-klotz
added
release
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 22520632 | Triggered | Generic High Entropy Secret | 29da1f3 | packages/core/credential/repositories/tests/credential-repository-documentdb-encryption.test.js | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
| const friggCommand = process.platform === 'win32' ? 'frigg.cmd' : 'frigg' | ||
|
|
||
| // Spawn the command | ||
| const childProcess = spawn(friggCommand, cmdArgs, { |
Check failure
Code scanning / SonarCloud
OS commands should not be vulnerable to command injection attacks High
| async executeFriggCommand(command, args = [], cwd = process.cwd()) { | ||
| return new Promise((resolve, reject) => { | ||
| const friggCli = path.join(__dirname, '../../../frigg-cli/index.js'); | ||
| const child = spawn('node', [friggCli, command, ...args], { |
Check failure
Code scanning / SonarCloud
I/O function calls should not be vulnerable to path injection attacks High
|
|
||
| try { | ||
| const url = `${this.npmRegistryUrl}/${packageName}`; | ||
| const response = await axios.get(url, { timeout: 10000 }); |
Check warning
Code scanning / SonarCloud
Server-side requests should not be vulnerable to traversing attacks Medium
|
|
||
| try { | ||
| const url = `${this.npmRegistryUrl}/${packageName}`; | ||
| const response = await axios.get(url, { timeout: 10000 }); |
Check warning
Code scanning / SonarCloud
Server-side requests should not be vulnerable to traversing attacks Medium
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef00b0f5a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+527
to
+530
| console.log('[authorize]: /api/authorize POST request details:'); | ||
| console.log('[authorize]: Headers:', JSON.stringify(req.headers, null, 2)); | ||
| console.log('[authorize]: Query params:', JSON.stringify(req.query, null, 2)); | ||
| console.log('[authorize]: Body:', JSON.stringify(req.body, null, 2)); |
There was a problem hiding this comment.
This route now logs full request headers, query params, and body for every /api/authorize POST, which can include bearer tokens, OAuth codes, client secrets, and other credentials; in environments where application logs are centralized or accessible to operators, this becomes an immediate secret-exposure path rather than transient debug output. Please gate this behind an explicit debug flag and redact sensitive fields before logging.
Useful? React with 👍 / 👎.
| const { code, state: returnedState } = await server.waitForCode(); | ||
|
|
||
| // 7. Verify state (CSRF protection) | ||
| if (returnedState && returnedState !== state) { |
There was a problem hiding this comment.
The callback validation only rejects when returnedState is present and mismatched, so callbacks that omit state entirely are accepted even though a random state was generated and sent. This weakens CSRF protection for the CLI OAuth flow because a local callback without state can still be processed as successful authorization.
Useful? React with 👍 / 👎.
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
4 participants
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.
No description provided.