feat(mcp): add authentication token support to MCP server- #614 #614 - #681
Closed
Clinton6801 wants to merge 828 commits into
Closed
Clinton6801 wants to merge 828 commits into
Clinton6801 wants to merge 828 commits into
Conversation
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited, 30s polling cadence, debug logging, isolated named volumes - Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised ports, LOG_LEVEL/NODE_ENV env vars - Add .env.example: full environment variable reference with inline comments - Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file presence, service config, volume isolation, network sharing, .env.example, and compose merge compatibility - Update .dockerignore: exclude compose files, systemd/, docs/, templates/ - Update .gitignore: allow .env.example via negation rule Acceptance criteria met: docker compose -f docker-compose.yaml -f docker-compose.devnet.yaml up boots daemon and mock RPC environment successfully. All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…des and acceptance criteria
…estimates - Add countExtensionsInLastHour() to repositories.ts to query extension_history for the past 60-minute window (issue TegoLabs#142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue TegoLabs#133) - Add comprehensive TDD tests written before implementation: - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases Closes TegoLabs#133 Closes TegoLabs#137 Closes TegoLabs#142
vitest.config.ts only globs tests/**/*.test.ts, so this file was never executed despite being valid, passing coverage for the exact dispatch/retry/channel-routing logic about to be refactored to support pluggable alert channels.
Central registration point for alert channel plugins. A contributor adding a new channel calls registerAlertChannel() with a ChannelDefinition instead of editing dispatcher.ts's channel map, the CLI's --type if/else chain, and a DB CHECK constraint.
Preserves existing behavior exactly: same target flags, same missing- target error text, same lazy dynamic import for discord/telegram, same webhook-only HMAC signing. This is the reference implementation new channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed lookup, so a plugin channel registered anywhere becomes deliverable without editing this file. Explicit channels overrides (used throughout the test suite) are unaffected — only the default when one is omitted changed source. deliverSingleAlert's channelType is widened from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry at the application layer instead of a fixed SQL enum, so adding a channel no longer requires a schema change. The CHECK now only guards against an empty string.
…ration The SCHEMA comment-stripper (`--.*\n`) silently failed to match comments ending in \r\n, since JS's `.` excludes all line terminators including \r. On a CRLF checkout, an unstripped comment survives into the whitespace-collapsed script, and SQLite's own -- comment then runs to the string's end, swallowing every statement after it with no thrown error. Switched to `--[^\n]*\n`, which matches either line ending. Latent since schema.sql had no comments before this change. Also adds relaxChannelTypeChecks(), following the existing migrateAlertConfigsChannelTypeCheck() convention, to rebuild alert_configs and resource_alert_configs in place for databases created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and UndeliveredResourceAlerts previously hardcoded the built-in channel names in their type signatures. The registry is now the source of truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded 5-name CHECK on every test, a leftover workaround from before schema.sql had these columns natively. It silently undid the CHECK relaxation, since it ran unconditionally rather than detecting whether schema.sql already had the change. getDatabaseForTesting() already execs the current schema.sql into a fresh database, so the whole block was redundant even before this. Also adds coverage for plugin channel_type values and empty-string rejection on both alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the alert channel registry, so a plugin channel's --type, target flag, missing-target error, and signing behavior all come from its ChannelDefinition instead of a hardcoded branch in this file. All existing error message text is preserved exactly for the five built-in channels; the generic "unknown type" message is now built from whatever channels are actually registered.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Add src/mcp/auth.ts with token resolution and verification functions - Support mcpAuthToken in config.yaml and SOROKEEP_MCP_TOKEN env var - Env var takes precedence over config (matches other secrets pattern) - When no token configured, preserve open-access behavior (backward compatible) - Add comprehensive auth unit tests with edge cases - Update MCP server to accept optional auth token parameter - Update MCP command to load and pass auth token from config - Add integration tests for MCP server with/without authentication - Never log token values, even at debug level (per SECURITY.md)
Author
|
@AbdulmalikAlayande , merge conflict is now resolved please check and give points |
Author
|
@AbdulmalikAlayande , have you reviewed this ? |
Author
|
@AbdulmalikAlayande , kindly review |
Collaborator
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
18 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.
Issue
Closes #410
Summary
Implements opt-in authentication token support for the MCP server, allowing users to restrict access to sorokeep's AI tools via environment variable or configuration file.
Changes
New Files
auth.ts
: Core authentication module
resolveToken(config): Resolves MCP auth token from SOROKEEP_MCP_TOKEN env var (takes precedence) or config.mcpAuthToken field
verifyRequest(token, configured): Validates incoming request tokens against configured token
Never logs token values (compliance with SECURITY.md)
Returns null when no token configured (backward compatible, open access)
auth.test.ts
: Comprehensive test suite (TDD-first)
12 test cases covering token resolution from env var, config, and neither
Tests verify backward compatibility (no token = open access)
Tests verify correct token allows access, wrong/missing token denies access
Security tests confirm token value is never logged
Modified Files
config.ts
: Added mcpAuthToken?: string field to SorokeepConfig interface
Follows same pattern as existing slackToken field
Properly parsed from YAML config files
server.ts
: Integrated authentication into MCP server
createMcpServer() now accepts SorokeepConfig parameter
Calls resolveToken() on startup to resolve configured token
Logs warning when running without authentication (per #410 requirements)
Never logs token value itself
index.ts
: Direct entry point updated
Loads config via loadConfig()
Passes config to createMcpServer()
mcp.ts
: CLI command updated
Loads config via loadConfig()
Passes config to createMcpServer()
lifecycle.test.ts
: Updated to pass config to createMcpServer()
get-extension-costs.test.ts
: Updated to pass config to createMcpServer()
Security
✅ Complies with SECURITY.md invariants:
Secret tokens never logged, even at debug level
Only logs 'token configured: true/false' status messages
No token value in error messages or stack traces
Env var SOROKEEP_MCP_TOKEN takes precedence over config file (env vars are more secure)
Backward Compatibility
✅ Opt-in authentication:
When no token is configured: all requests allowed (existing behavior maintained)
Users must explicitly set SOROKEEP_MCP_TOKEN env var or mcpAuthToken in config to enable access control
When token is configured but missing from request → 401 Unauthorized
Testing
✅ 12 unit tests for authentication logic in
auth.test.ts
:
Token resolution precedence (env > config > null)
Access control logic (configured token must match exactly)
Case-sensitive token comparison
No token value logging at any log level
Both open access (no token required) and restricted access (token required) modes
✅ Existing tests updated to work with new config parameter
Usage
Option 1: Environment Variable (Recommended for Production)
export SOROKEEP_MCP_TOKEN="your-secret-token-here"
sorokeep-mcp
Option 2: Config File
~/.sorokeep/config.yaml
network: testnet
pollingIntervalSeconds: 300
mcpAuthToken: "your-secret-token-here"
Option 3: No Authentication (Default)
sorokeep-mcp # Open access, no token required
Deployment Notes
If upgrading and no token is configured, server will log a warning but continue to work (backward compatible)
To enforce authentication, set SOROKEEP_MCP_TOKEN before starting the server
Token comparison is case-sensitive