Skip to content

Prevents APS login scope escalation#6

Merged
pabloderen merged 1 commit intomainfrom
hotfix/ScopeEscalation
Feb 24, 2026
Merged

Prevents APS login scope escalation#6
pabloderen merged 1 commit intomainfrom
hotfix/ScopeEscalation

Conversation

@pabloderen
Copy link
Collaborator

@pabloderen pabloderen commented Feb 24, 2026

Removes the ability to specify OAuth scopes for APS 3-legged login via an input argument. The login scope is now strictly determined by the APS_SCOPE environment variable, defaulting to data:read if not set.

This prevents granting excessive permissions and ensures better control over requested scopes.

Summary by CodeRabbit

  • Changes
    • APS login scope is now controlled globally through the APS_SCOPE setting, with a default of "data:read".
    • Per-call scope customization for APS login has been removed.

Removes the ability to specify OAuth scopes for APS 3-legged login via an input argument. The login scope is now strictly determined by the `APS_SCOPE` environment variable, defaulting to `data:read` if not set.

This prevents granting excessive permissions and ensures better control over requested scopes.
@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

Walkthrough

The PR simplifies the aps_login tool's scope handling in src/index.ts by removing the scope parameter from the input schema and constraining scope determination to exclusively use the APS_SCOPE user setting or default to "data:read". The tool description was updated to reflect this change.

Changes

Cohort / File(s) Summary
APS Login Tool Scope Simplification
src/index.ts
Removed scope property from aps_login input schema; reworked scope logic in handleTool to use only APS_SCOPE environment variable or default to "data:read"; updated tool description to mention APS_SCOPE-driven scope configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 The rabbit rejoices with glee,
One scope to set the OAuth free!
No more confusion, no more fuss,
APS_SCOPE guides us,
Simpler defaults, cleaner code,
Hop-hop-hop down the streamlined road! 🎉

🚥 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 'Prevents APS login scope escalation' directly and clearly describes the main change: removing the ability to specify OAuth scopes via input arguments to prevent scope escalation.
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 docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/ScopeEscalation

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

Copy link

@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.

🧹 Nitpick comments (1)
src/index.ts (1)

145-155: Disallow extra args in aps_login schema.

With empty properties, callers can still pass undeclared fields (e.g., scope) unless additionalProperties is set to false. Tightening this avoids confusion and reinforces the “no per-call scope” rule.

♻️ Proposed tweak
     inputSchema: {
       type: "object" as const,
       properties: {},
+      additionalProperties: false,
     },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 145 - 155, The aps_login command's inputSchema
currently allows undeclared fields because it defines type: "object" with empty
properties; update the schema for aps_login (inputSchema) to explicitly disallow
extra arguments by adding additionalProperties: false so callers cannot pass
fields like scope per-call. Modify the inputSchema object in the aps_login
definition to include additionalProperties: false while keeping type and
properties unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/index.ts`:
- Around line 145-155: The aps_login command's inputSchema currently allows
undeclared fields because it defines type: "object" with empty properties;
update the schema for aps_login (inputSchema) to explicitly disallow extra
arguments by adding additionalProperties: false so callers cannot pass fields
like scope per-call. Modify the inputSchema object in the aps_login definition
to include additionalProperties: false while keeping type and properties
unchanged.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 727868c and 1748c4d.

📒 Files selected for processing (1)
  • src/index.ts

@pabloderen pabloderen merged commit 7236a06 into main Feb 24, 2026
2 checks passed
@pabloderen pabloderen deleted the hotfix/ScopeEscalation branch February 24, 2026 19:58
@github-actions
Copy link

🎉 This PR is included in version 1.2.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

2 participants