Skip to content

fix(security): require authentication for profiler endpoints#366

Closed
nihalnihalani wants to merge 2 commits intorocketride-org:developfrom
nihalnihalani:fix/profiler-auth-bypass
Closed

fix(security): require authentication for profiler endpoints#366
nihalnihalani wants to merge 2 commits intorocketride-org:developfrom
nihalnihalani:fix/profiler-auth-bypass

Conversation

@nihalnihalani
Copy link
Copy Markdown
Contributor

@nihalnihalani nihalnihalani commented Mar 22, 2026

Summary

  • Remove public=True from all 5 profiler endpoint registrations to enforce authentication
  • Prevents unauthenticated access to profiling controls and performance data

Bug Description

File: packages/ai/src/ai/modules/profiler/__init__.py (lines 152–156)
Severity: High — Information Disclosure + Denial of Service

All 5 profiler endpoints are registered with public=True, which tells the AuthMiddleware to skip authentication:

server.add_route('/profile', get_profile_dashboard, ['GET'], public=True)
server.add_route('/profile/start', start_profiling, ['POST'], public=True)
server.add_route('/profile/stop', stop_profiling, ['POST'], public=True)
server.add_route('/profile/status', get_profile_status, ['GET'], public=True)
server.add_route('/profile/report', get_profile_report, ['GET'], public=True)

This means any unauthenticated user can:

1. Information Disclosure

  • GET /profile/report returns a full profiling report containing:
    • Internal Python function names and module paths
    • Call counts for each function
    • Cumulative and per-call timing data
    • Call hierarchy and relationships
  • GET /profile/status reveals session history and runtime statistics
  • GET /profile serves an interactive HTML dashboard with all data

This is valuable reconnaissance data — an attacker learns internal function names, hot paths, and performance characteristics before launching targeted attacks.

2. Denial of Service

  • POST /profile/start enables cProfile, which adds measurable CPU overhead to every function call in the Python process
  • An attacker can repeatedly start profiling sessions, degrading server performance
  • POST /profile/stop can interrupt legitimate profiling sessions

Attack Scenario

# Attacker (no credentials required):

# 1. Start profiling to degrade performance
curl -X POST http://target:5565/profile/start?session=recon

# 2. Wait for interesting activity...

# 3. Stop and read the report
curl -X POST http://target:5565/profile/stop
curl http://target:5565/profile/report

# Report reveals:
#   ai.common.agent._internal.host.Memory.put  - 1247 calls, 3.2s
#   ai.modules.chat.chat._handle_message       - 89 calls, 12.1s
#   ai.web.middleware.AuthMiddleware.__call__   - 4521 calls, 0.8s
# → Attacker now knows internal function paths and performance bottlenecks

Root Cause

The profiler module was likely registered as public during development for easy debugging and the public=True flags were never removed before release.

Fix

Remove public=True from all 5 route registrations. Without this flag, the AuthMiddleware (defined in packages/ai/src/ai/web/middleware.py) requires authentication for these endpoints — the same protection applied to all other non-public routes.

No new code needed — just removing the bypass flag.

Verification

  • ruff check passes
  • ruff format passes
  • All pre-commit hooks pass
  • Profiler endpoints now require authentication (same as /use, /task, etc.)
  • Only /redoc and /openapi.json remain as intentionally public endpoints

References

#frontier-tower-hackathon

Summary by CodeRabbit

  • Bug Fixes
    • Profiler endpoints now require authentication and are no longer publicly accessible; profiling controls and report access are restricted to authenticated users.
  • Documentation
    • Clarified that start/stop profiling actions use POST and updated status reporting of those control endpoints.

All 5 profiler endpoints (/profile, /profile/start, /profile/stop,
/profile/status, /profile/report) were registered with public=True,
bypassing the AuthMiddleware entirely. This allows unauthenticated
users to start/stop profiling and read performance reports that expose
internal function names, call counts, and timing data.

Remove public=True from all profiler route registrations so they
require authentication like other protected endpoints.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fe1148cb-64c8-4304-8a0f-c734d53db669

📥 Commits

Reviewing files that changed from the base of the PR and between 7753364 and 12b59e2.

📒 Files selected for processing (1)
  • packages/ai/src/ai/modules/profiler/__init__.py

📝 Walkthrough

Walkthrough

Removed public=True from five profiler route registrations (/profile, /profile/start, /profile/stop, /profile/status, /profile/report), so access now relies on the server’s default auth. Updated doc/comments to reflect that start/stop endpoints use POST and adjusted status labels from (PUT) to (POST).

Changes

Cohort / File(s) Summary
Profiler Route Access Control
packages/ai/src/ai/modules/profiler/__init__.py
Removed public=True from /profile, /profile/start, /profile/stop, /profile/status, /profile/report route registrations. Updated module comments/docs to state start/stop use POST and corrected reported method labels in get_status() from (PUT) to (POST).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudged five doors from open to shut,
Profiles now ask who’s coming, no clutter or rut,
POSTs take the lead, labels set right,
Quiet and tidy through day and night,
A hopping cheer for safer input! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: removing the public flag from profiler endpoints to enforce authentication. It directly reflects the primary purpose of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/ai/src/ai/modules/profiler/__init__.py`:
- Around line 155-156: The docs and status listing incorrectly state the
profiler endpoints use PUT while the actual routes use POST; update the usage
header text and the get_status() returned endpoint list to show '/profile/start'
and '/profile/stop' as POST methods and ensure any docstring or header
mentioning start_profiling and stop_profiling reflects POST instead of PUT so
consumers see the correct HTTP method.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a30f3b43-1510-437c-86c1-c10c34fea3dc

📥 Commits

Reviewing files that changed from the base of the PR and between 69894a2 and 7753364.

📒 Files selected for processing (1)
  • packages/ai/src/ai/modules/profiler/__init__.py

Comment thread packages/ai/src/ai/modules/profiler/__init__.py
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@asclearuc
Copy link
Copy Markdown
Collaborator

PR #362 has the same changes (almost), plus unit tests.
Next lines are unique to this PR https://github.com/rocketride-org/rocketride-server/pull/366/changes#diff-1d8509e8f3803840da889e5a1937812d012b724e8dbb54dc5ca896efb6d1cc44R173-R174 and should be used in #362

@kwit75
Copy link
Copy Markdown
Collaborator

kwit75 commented Mar 23, 2026

Hi @nihalnihalani, thanks for the security fix here! It looks like @heatxsink opened #362 for the same profiler auth bypass, and that PR also includes HTML escaping for the /profile/report endpoint plus a full test suite. To keep discussion centralized, marking this as a duplicate in favor of #362.

We'd love to have your eyes on other open issues — there are still several security findings that need attention!

heatxsink added a commit to heatxsink/rocketride-server that referenced this pull request Mar 23, 2026
Aligns documentation with actual route registration (POST, not PUT).
Incorporates fix from rocketride-org#366.
@asclearuc
Copy link
Copy Markdown
Collaborator

Closing this PR in favor of #362

@asclearuc asclearuc closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists module:ai AI/ML modules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants