Allowed methods middleware is

[dev-jodee]

now separate instead of only being available when auth is enabled.

---

Important

Separated allowed methods middleware from authentication, introducing MethodValidationLayer for independent method validation.

• Middleware Changes:
  • Introduced MethodValidationLayer in middleware_utils.rs to handle allowed methods independently.
  • Removed allowed methods logic from ApiKeyAuthLayer and HmacAuthLayer in auth.rs.
• Server Configuration:
  • Updated run_rpc_server in server.rs to include MethodValidationLayer for method validation.
• Tests:
  • Adjusted tests in auth.rs and basic_endpoints.rs to reflect the separation of method validation from authentication.
  • Added tests for MethodValidationLayer in middleware_utils.rs.

^{This description was created by for 9ab2483. You can customize this summary. It will automatically update as commits are pushed.}

---

📊 Unit Test Coverage

Unit Test Coverage: 83.6%

View Detailed Coverage Report

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 82e87dd in 1 minute and 35 seconds. Click for details.

• Reviewed 606 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. crates/lib/src/rpc_server/auth.rs:15

• Draft comment:
  Good removal of the allowed_methods field to decouple auth from method validation.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions. It simply acknowledges a change made in the code without offering any constructive criticism or questions.

2. crates/lib/src/rpc_server/auth.rs:65

• Draft comment:
  Clear bypass for the 'liveness' method is applied using get_jsonrpc_method; this consistent check in both API key and HMAC services ensures low-latency health checks.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the implementation without offering any constructive criticism or questions.

3. crates/lib/src/rpc_server/auth.rs:75

• Draft comment:
  Using unwrap_or("") for header conversion may hide errors; consider logging or handling conversion failures explicitly.
• Reason this comment was not posted:
  Comment was on unchanged code.

4. crates/lib/src/rpc_server/middleware_utils.rs:48

• Draft comment:
  The new MethodValidationLayer cleanly checks allowed methods and responds with UNAUTHORIZED on invalid requests.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the implementation without offering any constructive criticism or questions.

5. crates/lib/src/rpc_server/server.rs:80

• Draft comment:
  Adding MethodValidationLayer into the middleware stack decouples method authorization from auth layers and is well-ordered before API key and HMAC authentication.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that the addition of MethodValidationLayer is a good change without offering any specific guidance or questions.

6. crates/lib/src/rpc_server/server.rs:80

• Draft comment:
  Typo: There is an extra space after '-' in the comment "Method validation layer - to fail fast". Consider reducing to a single space (e.g., "Method validation layer - to fail fast").
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the comment is technically correct about the extra space, it's an extremely minor formatting issue in a comment. Comments about pure formatting, especially in comments themselves, are not valuable enough to warrant attention in a code review. This falls under the "Do NOT make comments that are obvious or unimportant" rule. The comment is factually accurate and suggests a specific fix. It could help maintain consistent formatting throughout the codebase. While consistent formatting is good, this level of nitpicking about spaces in comments doesn't add meaningful value to the code review and could distract from more important issues. Delete this comment as it's too minor and doesn't improve code quality in any meaningful way.

Workflow ID: wflow_KFiEEt7I6WAt4hfh

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[amilz]

Ya, imo this is better!

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 9ab2483 in 50 seconds. Click for details.

• Reviewed 31 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. Makefile:17

• Draft comment:
  Including 'build' in the 'test-all' target ensures artifacts are compiled before testing. Confirm that this added step is intentional as it may slow down quick test iterations.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. tests/rpc/basic_endpoints.rs:140

• Draft comment:
  The test now asserts that a disabled method returns an error containing '405'. Consider verifying the error code directly if possible to avoid brittle string matching.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_FfH04yT8zEmKN7Tw

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 92ba0f7 in 43 minutes and 53 seconds. Click for details.

• Reviewed 47 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. crates/lib/src/rpc_server/middleware_utils.rs:102

• Draft comment:
  Good change: the middleware now returns METHOD_NOT_ALLOWED (405) instead of UNAUTHORIZED (401) for invalid JSONRPC methods. Consider including an Allow header if HTTP semantics for 405 are desired.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. crates/lib/src/rpc_server/middleware_utils.rs:156

• Draft comment:
  Test assertions updated to expect METHOD_NOT_ALLOWED are consistent with the middleware change.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_AXinVcZ1i6nkkl9i

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}