Skip to content

Conversation

@isaacharrisholt
Copy link
Contributor

No description provided.

@rover-app
Copy link

rover-app bot commented Jun 14, 2025

Hey @isaacharrisholt!

We're scanning your PR for issues. Stand by for comments.

Lots of love,
Rover 🤖

Copy link

@rover-app rover-app bot left a comment

Choose a reason for hiding this comment

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

Rover alert scan for Pevensie

Scanned on Jun 14th 2025, 07:22PM for commit f2f782eae2d44be6056a4b45797b07ce2c298d4a

Rover found 3 risks.
3 medium.

Identified potential security vulnerabilities and bugs including a potential assertion crash, missing error handling, and potential sensitive information exposure.

Other Issues

We could not match the following issues to corresponding files within the code changes

Assertion crash on non-ProcessDown monitor message

Severity medium

Description The assertion in the `CallerDown` handler assumes a specific message type without proper error handling, which could cause the pool to crash if an unexpected message is received.

Suggested fix

      // We don't monitor ports
      case process_down {
        process.ProcessDown(pid: process_down_pid, ..) -> {
          // noop
        }
        _ -> { // TODO: Remove this when gleam allows empty case statements
          logging.debug("Unexpected message in CallerDown handler")
        }
      }

Potential sensitive information exposure in error logs

Severity medium

Description Direct logging of user-provided error messages may expose sensitive information in system logs.

Suggested fix

Sanitize the resource_create_error string before logging it to prevent potential sensitive information exposure. For example, redact or remove any user-provided data from the error message before logging.


Missing error handling for shutdown_resource

Severity medium

Description Lack of error handling around user-provided shutdown functions could cause pool actor crashes when a client process terminates.

Suggested fix

          result.try(fn() { Ok(state.shutdown_resource(live_resource.resource)) })
          |> result.map_error(fn(error) {
            logging.log(
              logging.Error,
              "Bath: Resource shutdown failed: " <> string.inspect(error),
            )
          })
          |> result.ok

To request another review, comment @rover-app review in the PR discussion.

Rover Support

Rover has scanned for issues in performance, security, reliability that might be introduced by this PR, in the context of your upstream and downstream services and dependencies.

What happens next

You can re-request a review by commenting @rover-app review on the PR.
Rover will review the PR again, and close any alerts that you've fixed.

I want to follow up with Rover

PR chat is coming

Soon, you'll be able to talk to Rover about issues in your PR, in your PR.
Right now, we only support code chat on your `main`/`master` (default) branch:
head to the graph page on the Rover platform
to chat with your code.

If Rover isn't doing much

It could be that Rover doesn't support your language or framework yet, or perhaps you've found an area we can improve in!
We'd love to get your feedback to help improve Rover, so if you're not happy with its output please get in touch by clicking here.

I love/hate the alerts Rover is generating

Regardless, we'd love to hear it!
We're working hard to make Rover better,
so please get in touch with us
with your PR number and alert comment.

I'd like to request a feature or improvement

You know the score: get in touch!
We love to have feature requests from our users to work on.

Rover actions

Re-review

Comment @rover-app review on the PR to request another review.

Suspend Rover scanning

To stop Rover from scanning PRs on your org (Pevensie), head to your organization settings or suspend the GitHub app installation on this GitHub account.

@isaacharrisholt isaacharrisholt merged commit bb6ecc0 into main Jun 14, 2025
1 check 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.

2 participants