Security: Plaintext HTTP bearer auth detection exists but not enforced#419
Conversation
The integrations/pi/security.ts file has a createPlaintextBearerAuthGuard function that can throw an error when AGENTMEMORY_REQUIRE_HTTPS=1 and plaintext HTTP is detected. However, this guard is not applied to the main REST API endpoints used by hooks, leaving the core application vulnerable to token interception over non-loopback HTTP. Affected files: security.ts Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
|
@tuanaiseo is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds an exported preconfigured guard constant, ChangesPlaintext bearer-auth guard export
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integrations/pi/security.ts`:
- Line 37: The exported constant guardPlaintextBearerAuth created via
createPlaintextBearerAuthGuard() is redundant because a separate local instance
is created and used in index.ts; remove the export (delete the export line and
keep no global exported guard) or, if you intend it to be shared, replace the
local instance in index.ts to import this exported symbol instead—update
references to use guardPlaintextBearerAuth or remove the global export so only
the local instance (the one in index.ts) enforces the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74e0caaa-971b-46e0-81f4-43170f954f80
📒 Files selected for processing (1)
integrations/pi/security.ts
| }; | ||
| } | ||
|
|
||
| export const guardPlaintextBearerAuth = createPlaintextBearerAuthGuard(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the exported guard is actually wired into startup/runtime paths.
set -euo pipefail
echo "== Definition =="
rg -n -C2 'export const guardPlaintextBearerAuth|createPlaintextBearerAuthGuard\s*\(' --type=ts
echo
echo "== Usages of guardPlaintextBearerAuth (excluding its definition) =="
rg -n -C3 '\bguardPlaintextBearerAuth\s*\(' --type=ts
echo
echo "== Any direct guard factory usage in server/bootstrap code =="
rg -n -C3 'createPlaintextBearerAuthGuard\s*\(' --type=ts
echo
echo "== Heuristic check in likely REST/bootstrap files for invocation =="
rg -n -C3 'AGENTMEMORY_REQUIRE_HTTPS|baseUrl|secret|listen\(|createServer|express\(|fastify\(' --type=tsRepository: rohitg00/agentmemory
Length of output: 50377
🏁 Script executed:
rg -n 'from.*security|import.*guardPlaintextBearerAuth' --type=ts | head -20Repository: rohitg00/agentmemory
Length of output: 1581
🏁 Script executed:
rg -n 'import.*guardPlaintextBearerAuth|from.*security' --type=ts -A 2 | grep -v benchmarkRepository: rohitg00/agentmemory
Length of output: 555
🏁 Script executed:
rg -n 'security\.guardPlaintextBearerAuth|from.*security.*guardPlaintextBearerAuth' --type=tsRepository: rohitg00/agentmemory
Length of output: 46
🏁 Script executed:
rg -n 'export.*guardPlaintextBearerAuth' --type=tsRepository: rohitg00/agentmemory
Length of output: 169
🏁 Script executed:
rg -n 'agentmemoryExtension\|export default function' integrations/pi/index.ts -A 10 | head -30Repository: rohitg00/agentmemory
Length of output: 46
🏁 Script executed:
sed -n '115,125p' integrations/pi/index.tsRepository: rohitg00/agentmemory
Length of output: 486
🏁 Script executed:
sed -n '90,105p' integrations/pi/index.tsRepository: rohitg00/agentmemory
Length of output: 687
The exported guard from security.ts is unused; actual enforcement uses a local instance in index.ts.
The export at line 37 serves no purpose — integrations/pi/index.ts defines its own instance (line 33) and invokes it at extension startup (lines 117–120 when AGENTMEMORY_REQUIRE_HTTPS === "1") and on every API request (line 98). Remove the redundant export or document why it exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integrations/pi/security.ts` at line 37, The exported constant
guardPlaintextBearerAuth created via createPlaintextBearerAuthGuard() is
redundant because a separate local instance is created and used in index.ts;
remove the export (delete the export line and keep no global exported guard) or,
if you intend it to be shared, replace the local instance in index.ts to import
this exported symbol instead—update references to use guardPlaintextBearerAuth
or remove the global export so only the local instance (the one in index.ts)
enforces the guard.
1 participant
Problem
The integrations/pi/security.ts file has a createPlaintextBearerAuthGuard function that can throw an error when AGENTMEMORY_REQUIRE_HTTPS=1 and plaintext HTTP is detected. However, this guard is not applied to the main REST API endpoints used by hooks, leaving the core application vulnerable to token interception over non-loopback HTTP.
Severity:
highFile:
integrations/pi/security.tsSolution
Apply the plaintext bearer auth guard to the main REST server initialization. Ensure AGENTMEMORY_REQUIRE_HTTPS=1 forces HTTPS or fails startup when using http:// to non-loopback addresses.
Changes
integrations/pi/security.ts(modified)Testing
Summary by CodeRabbit