Skip to content

Improve permissions checking#1696

Merged
epugh merged 10 commits into
mainfrom
improve_permissions_checking
May 12, 2026
Merged

Improve permissions checking#1696
epugh merged 10 commits into
mainfrom
improve_permissions_checking

Conversation

@epugh
Copy link
Copy Markdown
Member

@epugh epugh commented May 2, 2026

Description

We need to scope lookups for data to who owns it instead of using :id directly.

Motivation and Context

Better security

How Has This Been Tested?

manual and tests

Types of changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • [] New feature (non-breaking change which adds new functionality)
  • [] Breaking change (fix or feature that would cause existing functionality to change)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens authorization by scoping record lookups to the current user (or the current book/team) instead of fetching by raw :id, reducing the chance of unauthorized access via ID guessing.

Changes:

  • Scope Team, ApiKey, Judgement, and QueryDocPair lookups through current_user / @book.
  • Scope API “share” endpoints to cases/scorers the current user is involved with, returning 404 when inaccessible.
  • Add/adjust tests to cover the new authorization behavior and reduce noisy output/flaky external LLM behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/services/llm_service_test.rb Removes debug output; makes the Ollama test conditional on reachability and ensures WebMock is restored.
test/controllers/teams_controller_test.rb Switches fixture user and adds coverage for non-member access/rename attempts.
test/controllers/api_keys_controller_test.rb Adds integration tests ensuring users can only delete their own API keys.
test/controllers/api/v1/team_scorers_controller_test.rb Adds a not-found test when sharing an inaccessible scorer.
test/controllers/api/v1/team_cases_controller_test.rb Adds a not-found test when sharing an inaccessible case.
app/controllers/teams_controller.rb Scopes set_team to current_user.teams.
app/controllers/query_doc_pairs_controller.rb Reorders callbacks and scopes query-doc-pair lookup via @book.
app/controllers/judgements_controller.rb Reorders callbacks and scopes judgement lookup via @book.
app/controllers/api_keys_controller.rb Scopes API key deletion via current_user.api_keys.
app/controllers/api/v1/team_scorers_controller.rb Scopes scorer lookup via current_user.scorers_involved_with.
app/controllers/api/v1/team_cases_controller.rb Scopes case lookup via current_user.cases_involved_with (with includes).
app/controllers/ai_judges_controller.rb Scopes team and AI-judge lookups through current_user.teams and @team.members.only_ai_judges.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 6 to 8
before_action :set_book
before_action :set_query_doc_pair, only: [ :show, :edit, :update, :destroy ]

Comment on lines 6 to 8
before_action :set_book
before_action :set_judgement, only: [ :show, :edit, :update, :destroy ]

@epugh epugh merged commit 440254a into main May 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants