-
Notifications
You must be signed in to change notification settings - Fork 3
feat: database schema for train-id and credential management (ADR-026) #144
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
Open
crystalin
wants to merge
29
commits into
main
Choose a base branch
from
feature/db-train-credential-management
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
β¦-026) This commit implements the foundation for database-backed credential management, moving account credentials and train configurations from filesystem to PostgreSQL. **Database Schema:** - accounts table: Store Anthropic account credentials (API keys, OAuth) - trains table: Store train configurations with Slack config - train_account_mappings: Many-to-many relationship with priority **Key Features:** - AES-256-GCM encryption for sensitive credentials - PBKDF2 key derivation (100K iterations) - SHA-256 hashing for client API keys - Idempotent migrations following ADR-012 patterns - Feature flag (USE_DATABASE_CREDENTIALS) for gradual rollout - Moved Slack configuration from accounts to trains level **Security:** - Application-level encryption with CREDENTIAL_ENCRYPTION_KEY - Database constraints prevent invalid credential types - Foreign key constraints with proper cascade rules - Comprehensive indexes for performance **Files Changed:** - Migration 013: Database schema creation - Migration 014: Data import from filesystem - Shared types: DatabaseAccount, DatabaseTrain, TrainAccountMapping - Encryption utilities: encrypt(), decrypt(), hashApiKey() - Config: Added credentials section with feature flag - Documentation: ADR-026, environment variables, .env.example **Breaking Changes:** None - feature flag defaults to false (filesystem mode) **Next Steps:** - Repository layer implementation - Dashboard management UI - Integration tests Implements the foundation for #ADR-026 π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
β¦Phase 2A-C) Implements the repository layer abstraction for credential storage as per ADR-026 and the train credential migration roadmap Phase 2. ## Changes **Phase 2A: Type Definitions & Interfaces** - Added IAccountRepository interface for account credential operations - Added ITrainRepository interface for train-specific operations - Both interfaces work with account names for backward compatibility **Phase 2B: Filesystem Repositories (Backward Compatibility)** - FilesystemAccountRepository: wraps existing credentials.ts logic - FilesystemTrainRepository: wraps existing client key file logic - 100% backward compatible with current filesystem-based storage - No Slack config or train-account mappings in filesystem mode **Phase 2C: Database Repositories (New Functionality)** - DatabaseAccountRepository: queries accounts table with encryption/decryption - DatabaseTrainRepository: handles train-account mappings with priority - OAuth refresh uses SELECT FOR UPDATE row locking for concurrency safety - Maps between account_name (used by AuthService) and account_id (database PK) **Phase 2D: Factory** - create-repositories.ts: feature flag switching between filesystem and database - Validates encryption key and database pool based on configuration ## Implementation Notes - All repositories implement the same interface for interchangeability - Database mode requires USE_DATABASE_CREDENTIALS=true and CREDENTIAL_ENCRYPTION_KEY - Filesystem mode is the default (USE_DATABASE_CREDENTIALS=false) - OAuth token refresh in database mode is transaction-safe with row locking - Maintains existing account name-based selection logic (ID-based refactor deferred) ## Testing - β TypeScript compilation successful - β ESLint checks passed - β³ Unit tests (pending in Phase 2E) - β³ Integration tests (pending in Phase 2E) ## Related - ADR-026: Database Credential Management - Roadmap: docs/04-Architecture/train-credential-migration-roadmap.md - Phase 1: PR #144 (database schema and migrations) π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
β¦Service (Phase 2D) Completes Phase 2D of the train credential migration roadmap by wiring the repository layer into the dependency injection container and updating AuthenticationService to use repositories. ## Changes **Container Integration** - Import createRepositories factory and repository interfaces - Instantiate repositories during container initialization - Inject repositories into AuthenticationService constructor - Add getAccountRepository() and getTrainRepository() getter methods - Clean up repositories on container disposal **AuthenticationService Updates** - Accept optional accountRepository and trainRepository in constructor options - Use accountRepository.listAccountNames() when available, fall back to filesystem - Call accountRepository.clearCache() in clearCaches() method - Maintains 100% backward compatibility when repositories not provided ## Behavior **With Repositories (USE_DATABASE_CREDENTIALS=true)** - AuthenticationService delegates account listing to DatabaseAccountRepository - Account credentials are fetched from PostgreSQL with decryption - OAuth token refresh uses row-level locking for concurrency safety **Without Repositories (USE_DATABASE_CREDENTIALS=false, default)** - AuthenticationService falls back to existing filesystem logic - No behavioral changes from current implementation - Complete backward compatibility maintained ## Testing - β TypeScript compilation successful - β³ Runtime testing (pending) - β³ Unit tests (pending in Phase 2E) - β³ Integration tests (pending in Phase 2E) ## Related - Phase 2A-C: Repository implementations (commit ac42415) - ADR-026: Database Credential Management - Roadmap: docs/04-Architecture/train-credential-migration-roadmap.md π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This was referenced Oct 5, 2025
β¦ential-management
β¦dentials Implements Phase 3 of ADR-026 migration plan - dashboard interface for managing accounts and trains stored in PostgreSQL. **New Features:** - Configuration page at /dashboard/configuration with tabs for Accounts and Trains - REST API endpoints at /api/credentials/* for CRUD operations - Runtime feature flag check (USE_DATABASE_CREDENTIALS=true required) - Conditional navigation link (only shown when feature is enabled) **Repository Layer:** - CredentialsRepository in shared package for dashboard CRUD operations - Write-only pattern for secrets (no reveal endpoint) - Encryption handled transparently using existing AES-256-GCM utils - Optimized listTrains() query using LEFT JOIN with array_agg (avoids N+1) **Security:** - Feature flag middleware blocks unauthorized access - All sensitive data encrypted before storage - UUID-based account IDs prevent collisions - FK constraints with CASCADE for referential integrity - Read-only mode supported via existing middleware **UI Components:** - Tabbed interface for Accounts and Trains management - HTMX-powered forms for create operations - Server-side validation with error feedback - Multi-select for train-account associations **Code Quality Improvements:** - Used crypto.randomUUID() for secure ID generation - Imported hashApiKey at top of file for consistency - Single-query optimization for train listing **Files Added:** - packages/shared/src/database/credentials-repository.ts - services/dashboard/src/middleware/feature-flag.ts - services/dashboard/src/routes/credentials-api.ts - services/dashboard/src/routes/configuration.ts **Follow-up Tasks:** - Add edit form partials for inline account/train editing - Consider centralizing getRepository() helper function π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixes module resolution error when running the migration script. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Migration scripts run directly with bun and need to import from the built dist folder instead of using package aliases. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixes two issues in the credential management UI: 1. **Edit buttons now work**: - Added /accounts/:id/edit handler with inline edit form - Added /trains/:id/edit handler with inline edit form - Forms swap the table row using HTMX for seamless editing - Cancel button reloads the full page to restore original view 2. **Dynamic form fields based on credential type**: - Account creation form now shows different fields based on type - API Key type: shows single password field for API key - OAuth type: shows fields for access token, refresh token, expires_at - Uses JavaScript onchange event to toggle field visibility - Proper labels and placeholders for each field type **Edit Form Features**: - Account edit: can change name and update API key (write-only) - Train edit: can change name, description, and account associations - Credential type is read-only in edit mode (security constraint) - Multi-select properly shows currently selected accounts **UX Improvements**: - Clear "leave empty to keep current" messaging for secret fields - Disabled fields shown with gray background for clarity - Form validation messages for required fields - Security notes about encryption displayed π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
**Changes:** 1. Remove train_name from trains table schema - Removed trainName from DatabaseTrain type - Updated repository queries to exclude train_name - Updated dashboard UI to remove name column - Created migration 015 to drop train_name column 2. Enforce OAuth/API key credential immutability - UpdateAccountInput interface only allows accountName and isActive - Removed credential modification logic from updateAccount method - Added clear documentation about credential immutability - Credentials must be deleted and recreated to update **Migration Safety:** - Migration 015 is idempotent with data loss warning - Initial schema (013) updated to exclude train_name - TypeScript compilation and build verified **Security:** - Credential immutability enforced at type level - Repository layer simplified with no credential update paths - Clear error guidance for users π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
**Changes:** - check-oauth-status.ts: Query accounts from database by ID or name - oauth-login.ts: Create new OAuth accounts directly in database using PKCE flow - oauth-refresh.ts: Refresh tokens by deleting old account and creating new one (immutable) - oauth-refresh-all.ts: Batch refresh all OAuth accounts in database with dry-run support **Implementation Notes:** - All scripts now require DATABASE_URL and CREDENTIAL_ENCRYPTION_KEY - OAuth credentials are immutable - refresh creates new account ID - Scripts support both account ID and account name lookup - Maintains compatibility with existing OAuth flow and token refresh logic - Removed filesystem dependencies from all OAuth management scripts **Breaking Changes:** - Scripts no longer accept credential file paths - OAuth refresh changes account IDs (train configs must be updated) π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
**Problem:** Proxy was always checking filesystem for credentials even when USE_DATABASE_CREDENTIALS=true was set, causing confusing error messages about missing credentials directory. **Solution:** - Updated main.ts to check USE_DATABASE_CREDENTIALS flag before running filesystem credential status check - When flag is true, skip filesystem check and show appropriate message - Prevents "No credential files found" error when using database mode **Changes:** - services/proxy/src/main.ts: Add conditional check for USE_DATABASE_CREDENTIALS π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implements dashboard UI for generating and revoking API keys within train detail pages. **Features:** - Train detail page at /dashboard/trains/:trainId with API key management - "Generate API Key" button with HTMX modal workflow - One-time key display with copy-to-clipboard functionality - Revoke buttons for each key with confirmation - Train list now links to detail pages **Backend:** - Database migration 016: adds is_generated, key_hash, revoked_at columns - generateApiKey() utility: generates sk-dash-api01-<random> format keys - Repository methods: countGeneratedKeysForTrain(), generateApiKeyForTrain(), revokeAccount(), getAccountsForTrain() - API endpoints: POST /accounts/generate, PATCH /accounts/:id/revoke, GET /trains/:id/accounts - Enforces limits: 10 keys per train, 50 globally **Security:** - AES-256-GCM encryption for key storage - SHA-256 hashing for key display (last 4 chars only) - Constant-time hash comparison (crypto.timingSafeEqual) - One-time plaintext key exposure - Soft deletion via revoked_at timestamp - Transaction-safe key generation with rollback **Code Review:** - Reviewed by Gemini Pro - Refactored BFF endpoint to call repository directly (eliminates localhost/auth bypass) - Applied cryptographic best practices π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixes import resolution issue where Bun couldn't resolve the shared package import. Inlines encrypt() and hashApiKey() functions directly in the migration file to avoid dependency on build artifacts. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Updates the generateApiKey() function to use the ptk_ prefix instead of sk-dash-api01-. Generated keys now follow the format: ptk_<random_string> The generation remains fully automatic - users only provide a descriptive name, and the system generates the cryptographically secure key. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixes "inconsistent types deduced for parameter $1" error by passing trainId twice in the parameters array instead of reusing $1 in the subquery. Changed: WHERE train_id = $1 to WHERE train_id = $3 Parameters: [trainId, accountId, trainId] π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Generated API keys are now properly added to the train's client_api_keys_hashed array, allowing them to be used for client authentication to the proxy. Previously, keys were only stored in accounts table but not added to the train's client authentication list, causing "No client API key configured" errors. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Remove AES-256-GCM encryption support for database-backed credentials to simplify deployment and reduce operational complexity. Credentials are now stored as plaintext in PostgreSQL, relying on infrastructure-level security. Changes: - Database schema: rename columns (api_key_encrypted β api_key, etc.) - Remove encrypt() and decrypt() functions from shared/utils/encryption.ts - Remove CREDENTIAL_ENCRYPTION_KEY environment variable and validation - Update all repositories to store/retrieve credentials as plaintext - Keep SHA-256 hashing for client API key authentication - Update migrations 013 and 014 (pre-deployment, safe to modify) Security Model: - Database access controls and network isolation - RDS encryption at rest, TLS in transit - Strict backup access controls - SHA-256 hashing preserved for client authentication Documentation: - ADR-026 updated with decision rationale and security trade-offs - environment-vars.md updated to reflect plaintext storage - .env.example updated with security notes This change was made before production deployment, ensuring no encrypted data exists that would require migration. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix critical bugs preventing database credential mode from working: 1. Add getClientApiKeysHashed() method to ITrainRepository interface 2. Implement method in DatabaseTrainRepository to query hashed keys from DB 3. Implement method in FilesystemTrainRepository for plaintext keys 4. Update AuthenticationService to use trainRepository instead of filesystem 5. Fix client-auth middleware to handle both hashed and plaintext keys Changes: - ITrainRepository: add getClientApiKeysHashed() method definition - DatabaseTrainRepository: query client_api_keys_hashed from trains table - FilesystemTrainRepository: read plaintext keys from JSON files - AuthenticationService: use trainRepository.getClientApiKeysHashed() when available - client-auth.ts: detect hash format and compare appropriately - Database mode: SHA256(token) == stored_hash - Filesystem mode: SHA256(token) == SHA256(plaintext_key) This enables client authentication to work with USE_DATABASE_CREDENTIALS=true, allowing generated API keys (ptk_*) to authenticate against database-stored SHA-256 hashes. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Completes Phase 5 of ADR-026 by completely removing filesystem credential support. All credentials are now exclusively managed in PostgreSQL database. BREAKING CHANGE: Filesystem credential storage has been removed. Database credentials are now required. Set DATABASE_URL environment variable and run migration 014 to import existing filesystem credentials to database before upgrading. Changes: - Removed FilesystemAccountRepository and FilesystemTrainRepository - Simplified create-repositories.ts to only return database repositories - Refactored AuthenticationService to use only database repositories - Removed USE_DATABASE_CREDENTIALS, ACCOUNTS_DIR, TRAIN_CLIENT_KEYS_DIR, CREDENTIALS_DIR environment variables - Updated dashboard to always show Configuration UI (no feature flag check) - Added DATABASE_URL validation in container initialization - Updated error messages to reference database instead of filesystem Migration Guide: 1. Ensure DATABASE_URL is configured 2. Run migration scripts/db/migrations/014-import-credentials-data.ts 3. Verify credentials imported successfully 4. Remove filesystem credential files from credentials/ directory 5. Remove deprecated environment variables from .env Security Model (unchanged): - Credentials stored as plaintext in PostgreSQL - Security via infrastructure: VPC isolation, database access controls, TLS, encryption at rest π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The scripts/auth/oauth-login.ts script requires readline-sync for interactive prompts, but it was missing from dependencies. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
HTMX sends form data as URL-encoded by default, but the API expects JSON. Added hx-ext="json-enc" to fix JSON Parse error when creating trains. Fixes error: JSON Parse error: Unexpected identifier "trainId" π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Train tokens (client API keys for authenticating TO the proxy) were incorrectly being stored as accounts in the accounts table. They are now stored directly in the trains.client_api_keys_hashed array where they belong. Changes: - Simplified generateApiKeyForTrain to only update trains.client_api_keys_hashed - Removed account creation and train_account_mappings insert - Updated API endpoint comments to clarify train tokens vs account credentials - Changed response format to use train_id and token (instead of account_id/api_key) This fixes the conceptual confusion between: - Account credentials: Anthropic API keys/OAuth (for requests TO Anthropic) - Train tokens: Client API keys (for requests TO the proxy service) π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1. Removed feature-flag.ts middleware (no-op after removing filesystem credentials) - Deleted services/dashboard/src/middleware/feature-flag.ts - Removed requireDbCredentials import and usage from app.ts - Credential management routes are now always available 2. Disabled train token hashing (store as plaintext) - Updated generateApiKeyForTrain to store plaintext tokens - Updated validateClientKey to compare plaintext (no hashing) - Removed hashApiKey import from DatabaseTrainRepository - Note: Despite column name 'client_api_keys_hashed', storing plaintext for now Rationale: Simplifies token management. Security relies on database access controls and TLS in transit (consistent with account credential storage model). π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Combine migrations 013, 015, and 016 into a single comprehensive migration for accounts, trains, and train_account_mappings schema. Changes: - Merge migration 013 (original schema) + 016 (API key generation) into single file - Rename migration 014 to 014-import-filesystem-credentials for clarity - Delete migration 015 (redundant - train_name column never existed) - Delete migration 016 (merged into 013) - Update init-database.sql to include complete credential management schema - Add comments clarifying plaintext storage (ADR-026) and client API key purpose This consolidation simplifies the migration history and makes it clear that the credential management schema is a single logical unit. π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Delete migrations that were consolidated into 013 and 014: - 014-import-credentials-data.ts (renamed to 014-import-filesystem-credentials.ts) - 015-drop-train-name-column.ts (redundant - column never existed) - 016-add-api-key-generation-support.ts (merged into 013) π€ Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
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.
Summary
Complete removal of filesystem credential storage and full transition to database-backed credential management (ADR-026) with simplified plaintext security model.
Key Changes
ποΈ Filesystem Credential Removal (Phase 5 - COMPLETE)
FilesystemAccountRepository,FilesystemTrainRepositoryAuthenticationServiceUSE_DATABASE_CREDENTIALS,ACCOUNTS_DIR,TRAIN_CLIENT_KEYS_DIRπ Database Schema (Consolidated)
π Security Model (Simplified)
π« Train Token Clarification
trains.client_api_keys_hashedarray (despite column name, stored plaintext)/api/credentials/accounts/generateadds tokens to train arrayπ Bug Fixes
hx-ext="json-enc"to train creation form)readline-syncdependency for OAuth login scriptποΈ Repository Pattern
π TypeScript Support
DatabaseAccount,DatabaseTrain,TrainAccountMapping,DecryptedAccounthashApiKey(),verifyApiKeyHash(),generateApiKey()Migration Consolidation
Before:
After:
014-import-filesystem-credentials.ts(clearer purpose)init-database.sqlwith complete schemaImplementation Status
β Phase 1: Database Schema
β Phase 2: Repository Layer
β Phase 3: Dashboard UI
β Phase 4: Integration
β Phase 5: Filesystem Removal (COMPLETE)
Testing
Documentation
Breaking Changes
DATABASE_URLrequired)Security Considerations
Current Model:
Train Token Security:
Future Enhancements:
Next Steps
π€ Generated with Claude Code