Skip to content

chore(release): staging to production - 2026.01.22 - #879

Merged
maxtechera merged 4 commits into
productionfrom
staging
Jan 22, 2026
Merged

chore(release): staging to production - 2026.01.22#879
maxtechera merged 4 commits into
productionfrom
staging

Conversation

@github-actions

@github-actions github-actions Bot commented Jan 22, 2026

Copy link
Copy Markdown

🚀 Release: Staging to Production

Release Date: 2026-01-22

Changes in this release

  • fix(AGENT-639): populate organizationId and userId when creating API keys (4d51edf)
  • fix(AGENT-639): add organizationId to queryVectorStore options (ad1c853)

This PR is automatically created/updated when commits are pushed to staging.
Merging this PR will trigger the release workflow to create a new GitHub release.

The previous fix changed userId/organizationId filtering to workspaceId
for finding the document store, but removed organizationId from the
options object. This broke vector retrieval because:

- Upsert stores documents in namespace: org:XXX_chatflow:YYY_default
- Query looked in namespace: chatflow:YYY_default (missing org prefix)

Adding organizationId from the entity ensures the namespace matches
and retrieval returns the correct chunks.

Fixes SUPPORT-17
…ore-workspace-filter

fix(AGENT-639): add organizationId to queryVectorStore for correct namespace
@vercel

vercel Bot commented Jan 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
answerai-docs Building Building Preview Jan 22, 2026 4:28pm
the-answerai Building Building Preview Jan 22, 2026 4:28pm

Request Review

…ore-workspace-filter

fix(AGENT-639): populate organizationId and userId when creating API keys
@claude

claude Bot commented Jan 22, 2026

Copy link
Copy Markdown

Pull Request Review: AGENT-639 - Multi-tenancy Fix for API Keys

Summary

This PR fixes a critical multi-tenancy issue where organizationId and userId were not being properly populated when creating API keys. The changes affect the API key creation, import, and vector store query flows.


✅ Positive Aspects

  1. Critical Security Fix: Properly populates organizationId and userId for API keys, which is essential for multi-tenancy and authorization
  2. Consistent Pattern: Changes align with the repository's 4-layer architecture pattern (Routes → Controllers → Services → Entities)
  3. Better Type Safety: Passes full IUser object instead of just workspace ID, making the data flow more explicit
  4. Complete Coverage: Fixes both the normal creation flow and the import flow

🔍 Code Quality Analysis

Controller Changes (packages/server/src/controllers/apikey/index.ts)

Good:

  • Properly passes req.user\! (non-null assertion after validation)
  • Consistent with existing validation pattern

Concern:

// Line 65-67: importKeys controller
req.body.workspaceId = req.user?.activeWorkspaceId
req.body.organizationId = req.user?.activeOrganizationId  // ⚠️ Optional chaining
req.body.userId = req.user?.id                             // ⚠️ Optional chaining

Issue: Uses optional chaining (?) even though req.user is already validated at line 62-64. These values could be undefined if the user object doesn't have them populated.

Recommendation: Add explicit validation:

if (\!req.user?.activeOrganizationId || \!req.user?.id) {
    throw new InternalFlowiseError(
        StatusCodes.PRECONDITION_FAILED, 
        `Organization ID and User ID are required`
    )
}
req.body.organizationId = req.user.activeOrganizationId
req.body.userId = req.user.id

Service Changes (packages/server/src/services/apikey/index.ts)

Good:

  • getAllApiKeys and createApiKey now accept IUser instead of just workspaceId (lines 31, 76)
  • Non-null assertions (\!) used appropriately after extracting from user object
  • Import function properly handles all three fields (workspaceId, organizationId, userId)

Architecture Note:
The service correctly populates all required fields in both scenarios:

  1. New key creation (lines 86-88)
  2. Key import (lines 207-208, 231-232)

Document Store Change (packages/server/src/services/documentstore/index.ts)

Good:

  • Adds organizationId to the options object for vector store queries
  • Uses the entity's organizationId ensuring proper multi-tenancy filtering

Question:
The change at line 1679 adds organizationId to the options passed to the vector store. This is good, but we should verify that downstream vector store implementations actually use this field for filtering.


🔒 Security Considerations

✅ Multi-tenancy Compliance

  • BEFORE: API keys were created without organizationId and userId, potentially causing data leaks
  • AFTER: All API keys now properly scoped to organization and user

⚠️ Potential Issues

  1. Null Safety in Import Flow:

    • Lines 133-134 extract organizationId and userId from body
    • No validation that these values are present before using them in lines 207-208, 231-232
    • If controller validation is bypassed somehow, these could be undefined
  2. Missing Database Index Check:

    • The ApiKey entity (line 22-26) has organizationId and userId columns
    • Should verify these are indexed for query performance (especially organizationId)

🧪 Test Coverage

Existing Tests

The PR doesn't add new tests, but existing test file exists:

  • packages/server/test/auth/apikey.test.ts - Tests API key authentication

Missing Test Coverage

Recommended additions:

describe('API Key Creation with Multi-tenancy', () => {
    it('should populate organizationId when creating API key', async () => {
        const key = await createTestApiKey(testUser)
        expect(key.organizationId).to.equal(testUser.activeOrganizationId)
    })
    
    it('should populate userId when creating API key', async () => {
        const key = await createTestApiKey(testUser)
        expect(key.userId).to.equal(testUser.id)
    })
    
    it('should populate organizationId and userId when importing keys', async () => {
        // Test import flow
    })
})

📋 Checklist Review (per .claude/rules/api-routes.md)

  • ✅ Controllers use proper error handling with InternalFlowiseError
  • ✅ Service methods follow naming conventions
  • ✅ Database queries filter by organizationId (via workspaceId relationship)
  • ⚠️ Missing explicit validation for organizationId and userId in import flow
  • ❓ Need to verify database indexes on organizationId and userId columns

🎯 Recommendations

High Priority

  1. Add validation in importKeys controller (Line 65-67):
if (\!req.user?.activeOrganizationId || \!req.user?.id) {
    throw new InternalFlowiseError(
        StatusCodes.PRECONDITION_FAILED,
        'Error: apikeyController.importKeys - organizationId and userId required'
    )
}
  1. Add validation in importKeys service (Line 133-134):
if (\!organizationId || \!userId) {
    throw new InternalFlowiseError(
        StatusCodes.PRECONDITION_FAILED,
        'Error: apikeyService.importKeys - organizationId and userId are required'
    )
}

Medium Priority

  1. Add tests for the new multi-tenancy fields in both create and import flows

  2. Verify database indexes exist for:

    • ApiKey.organizationId
    • ApiKey.userId
  3. Check vector store implementations to ensure they use the organizationId in options for filtering

Low Priority

  1. Consider refactoring updateApiKey to follow the same pattern (accept IUser instead of workspaceId)

🏁 Conclusion

Overall Assessment: ✅ APPROVE with recommendations

This is a critical security fix that properly implements multi-tenancy for API keys. The core logic is sound and follows repository conventions. The main concerns are around defensive validation to prevent edge cases where required fields might be undefined.

Severity of Issues Found:

  • 🟡 Medium: Missing null checks in import flow (should be fixed before merge)
  • 🟢 Low: Missing test coverage (can be added post-merge)
  • 🟢 Low: Database index verification (verify but likely already exists)

Recommendation: Add the suggested validation checks in the import flow, then this is ready to merge.


Review completed by: Claude Code Agent
Ticket: AGENT-639

@maxtechera
maxtechera merged commit 5631c76 into production Jan 22, 2026
13 of 14 checks passed
@maxtechera
maxtechera temporarily deployed to staging - aai-unified2-flowise-moonstruck January 22, 2026 17:42 — with Render Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant