Add Inboxes API #80
Add Inboxes API #80
Conversation
📝 WalkthroughWalkthroughAdds a new Inboxes API client (Mailtrap::InboxesAPI), wires it into the library, includes example usage, and provides comprehensive RSpec + VCR test fixtures covering success and error paths for inbox CRUD and mailbox operations. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant InboxesAPI as Mailtrap::InboxesAPI
participant HTTP as mailtrap.io API
participant Mapper as Response → Inbox
Client->>InboxesAPI: call create/update/get/clean/...
InboxesAPI->>HTTP: build path & send HTTP request
HTTP-->>InboxesAPI: return HTTP response (200/4xx/5xx)
InboxesAPI->>Mapper: handle_response / map JSON to Inbox
Mapper-->>Client: return Mailtrap::Inbox or raise error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
spec/mailtrap/inbox_spec.rb (1)
16-27: Consider using realistic data types for test attributes.Based on the VCR cassettes, several attributes have different types in the actual API:
usedshould be a boolean (e.g.,false), not an integersmtp_portsshould be an array (e.g.,[25, 465, 587, 2525])pop3_portsshould be an array (e.g.,[1100, 9950])Using representative data types would make the tests more realistic and could catch type-related issues in the Inbox model.
lib/mailtrap/inboxes_api.rb (2)
28-39: Documentation missingproject_idoption.The
createmethod requiresproject_idto construct the API path, but it's not documented in the YARD@optiontags.🔎 Proposed fix
# Creates a new inbox # @param [Hash] options The parameters to create # @option options [String] :name The inbox name + # @option options [Integer] :project_id The project identifier (required) # @return [Inbox] Created inbox object
57-59: Restrict updatable options to excludeproject_id.The
updatemethod currently uses the class-levelsupported_optionswhich includes:project_id, but according to the Mailtrap API documentation, the update endpoint only acceptsnameandemail_username. Theproject_idis only valid during inbox creation (to specify the target project) and cannot be updated. Thebase_updatemethod supports passing a restricted options list, so you can enforce this at the validation layer:+ UPDATE_OPTIONS = %i[name email_username].freeze + # Updates an existing Inbox # @param inbox_id [Integer] The Inbox identifier # @param [Hash] options The parameters to update # @option options [String] :name The inbox name # @option options [String] :email_username The inbox email username # @return [Inbox] Updated Inbox object # @!macro api_errors # @raise [ArgumentError] If invalid options are provided def update(inbox_id, options) - base_update(inbox_id, options) + base_update(inbox_id, options, UPDATE_OPTIONS) end
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
lib/mailtrap.rblib/mailtrap/inboxes_api.rbspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxe_objects.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.ymlspec/mailtrap/inbox_spec.rbspec/mailtrap/inboxes_api_spec.rb
🧰 Additional context used
🧬 Code graph analysis (2)
spec/mailtrap/inboxes_api_spec.rb (2)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)lib/mailtrap/inboxes_api.rb (9)
list(16-18)include(7-97)get(24-26)create(34-39)update(57-59)clean(65-68)mark_as_read(74-77)reset_credentials(83-86)delete(45-47)
lib/mailtrap/inboxes_api.rb (3)
lib/mailtrap/base_api.rb (7)
supported_options(27-29)response_class(31-33)base_list(67-70)base_get(46-49)validate_options!(35-40)base_delete(63-65)base_update(57-61)lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)lib/mailtrap/client.rb (2)
post(187-194)patch(201-208)
🪛 ast-grep (0.40.3)
lib/mailtrap/inboxes_api.rb
[warning] 35-36: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes",
wrap_request(options))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (21)
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This VCR cassette properly records a 404 Not Found scenario for updating a non-existent inbox. The Bearer token is appropriately masked, and the fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful delete operation returning the deleted inbox data. The fixture structure is standard, and sensitive data is appropriately masked.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This cassette properly records a 404 Not Found scenario for deleting a non-existent inbox. The fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This cassette properly records a 404 Not Found scenario for resetting credentials on a non-existent inbox. The fixture structure is standard.
lib/mailtrap.rb (1)
14-14: LGTM!The require statement follows the established pattern for loading API modules and is correctly placed in the sequence.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxe_objects.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful list operation returning an array of inbox objects. The fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful reset_credentials operation. The fixture structure is standard, and the response includes the updated inbox object with new credentials.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a 401 Unauthorized scenario for an incorrect API token. The fixture structure is standard and includes the expected
WWW-Authenticateheader.spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/returns_nil.yml (1)
1-168: LGTM! Standard VCR cassette for the clean endpoint.This fixture appropriately records the PATCH request to clean an inbox and the 200 OK response. Bearer tokens are properly masked, and the response structure aligns with the Mailtrap Inboxes API.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Appropriate error case fixture.This cassette correctly captures the 404 Not Found response when attempting to mark a non-existent inbox as read. The error handling test coverage is valuable.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Consistent error handling fixture.This cassette appropriately tests the 404 Not Found scenario for the clean endpoint when the inbox doesn't exist. Error handling coverage is consistent with other operations.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)
1-167: LGTM! Validation error fixture for create endpoint.This cassette captures a 422 Unprocessable Entity response when the sandbox limit is reached. The fixture appropriately tests API error handling with a meaningful error message.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.yml (1)
1-168: LGTM! Standard success case for mark_as_read.This cassette correctly records the PATCH request to mark all inbox messages as read and the 200 OK response. The response structure is consistent with other successful operations.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.yml (1)
1-168: LGTM! Comprehensive fixture for inbox retrieval.This cassette appropriately captures the GET request to retrieve an inbox and includes a complete inbox payload for testing object mapping. The fixture supports validation of the Mailtrap::Inbox object initialization.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Error handling for non-existent inbox.This cassette correctly captures the 404 Not Found response when attempting to retrieve a non-existent inbox. The error handling test coverage is complete.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.yml (1)
1-168: LGTM! Success case for inbox creation.This cassette appropriately captures the POST request to create an inbox and the response with complete inbox data. The fixture supports testing object mapping for newly created inboxes.
Note: The API returns 200 OK for creation instead of the more conventional 201 Created, but this accurately reflects the actual API behavior.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.yml (1)
1-168: VCR cassette looks good.The fixture properly records the PATCH request for updating an inbox with a hash request containing only the
namefield. The authorization token is correctly sanitized with a placeholder<BEARER_TOKEN>.spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.yml (1)
1-168: VCR cassette is correctly structured.The fixture properly captures the PATCH request for updating an inbox with both
nameandemail_usernamefields. Authorization token is appropriately sanitized.spec/mailtrap/inboxes_api_spec.rb (1)
1-236: Comprehensive test coverage for the InboxesAPI.The test suite covers all CRUD operations, inbox management methods (clean, mark_as_read, reset_credentials), and error handling scenarios including authorization errors and not found cases. Good use of VCR for HTTP interaction recording.
lib/mailtrap/inboxes_api.rb (2)
35-38: Static analysis false positive - no hardcoded credentials here.The ast-grep warning about hardcoded credentials is a false positive. This is simply an API endpoint URL constructed from dynamic parameters (
account_id,project_id). No actual secrets or passphrases are present in this code.
6-97: Well-structured API implementation.The
InboxesAPIclass follows the established patterns fromBaseAPI, with clean method implementations for CRUD and inbox management operations. The private helpers (wrap_request,base_path) are appropriately scoped, and the YARD documentation provides good API reference material.
examples/inboxes_api.rb
Outdated
| inboxes = Mailtrap::InboxesAPI.new(account_id, client) | ||
|
|
||
| # Create new Inbox | ||
| list = inboxes.create(name: 'Test Inbox', project_id: 123_456) |
lib/mailtrap/inboxes_api.rb
Outdated
| class InboxesAPI | ||
| include BaseAPI | ||
|
|
||
| self.supported_options = %i[name email_username project_id] |
lib/mailtrap/inboxes_api.rb
Outdated
| # @!macro api_errors | ||
| # @raise [ArgumentError] If invalid options are provided | ||
| def create(options) | ||
| validate_options!(options, supported_options) |
There was a problem hiding this comment.
validate_options!(options, supported_options + [:project_id])
| - W/"b806febaa0d1508c418fc9aa248e543f" | ||
| Cache-Control: | ||
| - max-age=0, private, must-revalidate | ||
| Content-Security-Policy: |
| - h3=":443"; ma=86400 | ||
| body: | ||
| encoding: ASCII-8BIT | ||
| string: '{"id":4278175,"name":"Updated Inbox","username":"9af1dc93bf734e","status":"active","email_username":"updated_username-e9d5fb","email_username_enabled":false,"used":false,"forward_from_email_address":"a2326475-i4278175@forward.mailtrap.info","project_id":2379735,"password":"b5bf6ce98dcfd9","domain":"sandbox.smtp.mailtrap.io","pop3_domain":"pop3.mailtrap.io","email_domain":"inbox.mailtrap.io","smtp_ports":[25,465,587,2525],"pop3_ports":[1100,9950],"api_domain":"sandbox.api.mailtrap.io","sent_messages_count":0,"forwarded_messages_count":0,"emails_count":0,"emails_unread_count":0,"last_message_sent_at":null,"max_size":10,"max_message_size":5242880,"permissions":{"can_read":true,"can_update":true,"can_destroy":true,"can_leave":false}}' |
spec/mailtrap/inbox_spec.rb
Outdated
| end | ||
| end | ||
|
|
||
| describe '#to_h' do |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@examples/inboxes_api.rb`:
- Around line 32-33: The comment above the inboxes.delete(inbox.id) call uses
incorrect terminology ("Delete contact list"); update that comment to accurately
read "Delete Inbox" (or similar correct wording) so it matches the operation
performed by the inboxes.delete(inbox.id) call.
- Around line 15-21: The comments incorrectly say "contact list" in the Inboxes
API example; update the comment lines above the calls to inboxes.update and
inboxes.get to refer to "inbox" (e.g., "Update inbox" and "Get inbox") and
ensure any surrounding wording matches the Inboxes API context (leave the calls
inboxes.update, inboxes.get and the id usage like list.id as-is unless variable
name should be inbox.id in this example).
In
`@spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxes_objects.yml`:
- Line 166: The VCR cassette JSON includes hardcoded sensitive fields (password,
username, email_username, forward_from_email_address, domains and any email
addresses) that must be redacted; update VCR configuration to call VCR.configure
{ |c| c.filter_sensitive_data('<PASSWORD>') { |i| extract password from
i.response.body } } and add similar filter_sensitive_data entries for username,
email addresses and domains, or manually edit the cassette JSON (the array with
keys "password", "username", "email_username", "forward_from_email_address",
"domain", "email_domain", "api_domain") replacing real values with placeholders
like "<PASSWORD>", "<USERNAME>", "<EMAIL>" so no real credentials remain.
♻️ Duplicate comments (2)
examples/inboxes_api.rb (1)
8-8: Rename variable for clarity.The variable
listholds a single created Inbox object, not a list. Consider renaming toinboxorcreated_inboxfor consistency with what the method returns.♻️ Suggested fix
# Create new Inbox -list = inboxes.create(name: 'Test Inbox', project_id: 123_456) +inbox = inboxes.create(name: 'Test Inbox', project_id: 123_456) # => #<struct Mailtrap::Inbox id=1, name="Test Inbox"> # Get all Inboxes inboxes.list # => [#<struct Mailtrap::Inbox id=1, name="Test Inbox">] -# Update contact list -inboxes.update(list.id, name: 'Test List Updated') +# Update Inbox +inboxes.update(inbox.id, name: 'Test Inbox Updated') # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> -# Get contact list -inbox = inboxes.get(list.id) +# Get Inbox by ID +inbox = inboxes.get(inbox.id) # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated">spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/cleans_Inbox_and_returns_Inbox_object.yml (1)
166-166: Sensitive data exposed in VCR cassette response body.The response body contains potentially sensitive information including
password,username, and email addresses (forward_from_email_address,email_username). These should be redacted similar to how the Bearer token is handled on line 19.Consider using VCR's
filter_sensitive_dataconfiguration or manually redacting these fields to prevent accidental credential exposure.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdexamples/inboxes_api.rbspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/cleans_Inbox_and_returns_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxes_objects.ymlspec/mailtrap/inbox_spec.rbspec/mailtrap/inboxes_api_spec.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/mailtrap/inboxes_api_spec.rb
- spec/mailtrap/inbox_spec.rb
🧰 Additional context used
🧬 Code graph analysis (1)
examples/inboxes_api.rb (2)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)lib/mailtrap/inboxes_api.rb (3)
clean(65-68)mark_as_read(74-77)reset_credentials(83-86)
🔇 Additional comments (1)
README.md (1)
183-183: LGTM!The documentation entry is correctly formatted and consistent with the existing entries. The link to the new example file is properly placed under the Email Sandbox section.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
examples/inboxes_api.rb
Outdated
| # Update contact list | ||
| inboxes.update(list.id, name: 'Test List Updated') | ||
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> | ||
|
|
||
| # Get contact list | ||
| inbox = inboxes.get(list.id) | ||
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> |
There was a problem hiding this comment.
Fix incorrect terminology in comments.
The comments reference "contact list" but this is the Inboxes API. These appear to be copy-paste artifacts from another example file.
📝 Suggested fix
-# Update contact list
+# Update Inbox
inboxes.update(list.id, name: 'Test List Updated')
# => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated">
-# Get contact list
+# Get Inbox by ID
inbox = inboxes.get(list.id)
# => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Update contact list | |
| inboxes.update(list.id, name: 'Test List Updated') | |
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> | |
| # Get contact list | |
| inbox = inboxes.get(list.id) | |
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> | |
| # Update Inbox | |
| inboxes.update(list.id, name: 'Test List Updated') | |
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> | |
| # Get Inbox by ID | |
| inbox = inboxes.get(list.id) | |
| # => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> |
🤖 Prompt for AI Agents
In `@examples/inboxes_api.rb` around lines 15 - 21, The comments incorrectly say
"contact list" in the Inboxes API example; update the comment lines above the
calls to inboxes.update and inboxes.get to refer to "inbox" (e.g., "Update
inbox" and "Get inbox") and ensure any surrounding wording matches the Inboxes
API context (leave the calls inboxes.update, inboxes.get and the id usage like
list.id as-is unless variable name should be inbox.id in this example).
examples/inboxes_api.rb
Outdated
| # Delete contact list | ||
| inboxes.delete(inbox.id) |
There was a problem hiding this comment.
Fix incorrect terminology.
The comment says "Delete contact list" but should say "Delete Inbox".
📝 Suggested fix
-# Delete contact list
+# Delete Inbox
inboxes.delete(inbox.id)🤖 Prompt for AI Agents
In `@examples/inboxes_api.rb` around lines 32 - 33, The comment above the
inboxes.delete(inbox.id) call uses incorrect terminology ("Delete contact
list"); update that comment to accurately read "Delete Inbox" (or similar
correct wording) so it matches the operation performed by the
inboxes.delete(inbox.id) call.
| - h3=":443"; ma=86400 | ||
| body: | ||
| encoding: ASCII-8BIT | ||
| string: '[{"id":3715083,"name":"My Inbox","username":"ef9d04e49208b5","status":"active","email_username":"e37fcfc109-57e921","email_username_enabled":false,"used":false,"forward_from_email_address":"a2326475-i3715083@forward.mailtrap.info","project_id":2379735,"password":"d0a5272456b7fb","domain":"sandbox.smtp.mailtrap.io","pop3_domain":"pop3.mailtrap.io","email_domain":"inbox.mailtrap.io","smtp_ports":[25,465,587,2525],"pop3_ports":[1100,9950],"api_domain":"sandbox.api.mailtrap.io","sent_messages_count":0,"forwarded_messages_count":0,"emails_count":0,"emails_unread_count":0,"last_message_sent_at":null,"max_size":10,"max_message_size":5242880,"permissions":{"can_read":true,"can_update":true,"can_destroy":true,"can_leave":false}}]' |
There was a problem hiding this comment.
Sensitive data exposed in VCR cassette response body.
Similar to other cassettes in this PR, the response body contains potentially sensitive information including password ("d0a5272456b7fb"), username, and email addresses. These should be redacted to prevent credential exposure.
💡 Recommendation
Configure VCR to filter sensitive response data:
VCR.configure do |config|
config.filter_sensitive_data('<PASSWORD>') do |interaction|
# Extract and redact password from response body
end
endOr manually redact the sensitive fields in the cassette files.
🤖 Prompt for AI Agents
In
`@spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxes_objects.yml`
at line 166, The VCR cassette JSON includes hardcoded sensitive fields
(password, username, email_username, forward_from_email_address, domains and any
email addresses) that must be redacted; update VCR configuration to call
VCR.configure { |c| c.filter_sensitive_data('<PASSWORD>') { |i| extract password
from i.response.body } } and add similar filter_sensitive_data entries for
username, email addresses and domains, or manually edit the cassette JSON (the
array with keys "password", "username", "email_username",
"forward_from_email_address", "domain", "email_domain", "api_domain") replacing
real values with placeholders like "<PASSWORD>", "<USERNAME>", "<EMAIL>" so no
real credentials remain.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/mailtrap/inboxes_api.rb`:
- Around line 28-39: The create method uses options[:project_id] in the path but
doesn't ensure it's present, so add an explicit validation that
options[:project_id] is provided (raise ArgumentError with a clear message if
missing or nil) before calling client.post; keep the existing validate_options!
call for supported keys but ensure project_id is treated as required by either
adding a presence check or adjusting validate_options! usage for required keys;
also update the YARD docs above create to include `@option` options [Integer,
String] :project_id (marked required) so callers know project_id is mandatory.
♻️ Duplicate comments (4)
examples/inboxes_api.rb (2)
19-21: Fix incorrect terminology in comment.The comment says "Get contact list" but should say "Get Inbox" to match the API being demonstrated.
32-33: Fix typo in comment.The comment says "Delete unbox list" which appears to be a typo. It should say "Delete Inbox".
📝 Suggested fix
-# Delete unbox list +# Delete Inbox inboxes.delete(inbox.id)spec/mailtrap/inboxes_api_spec.rb (2)
167-172: Misleading test description: method returns an Inbox object, not nil.The test description says "returns nil" but the expectation verifies an object with
idandnameattributes.
192-197: Misleading test description: method returns an Inbox object, not nil.The test description says "returns nil" but the expectation verifies an object with
idandnameattributes.
🧹 Nitpick comments (3)
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.yml (1)
75-77: Consider sanitizing SMTP credentials in the response body.The response body contains an SMTP
passwordfield with value"b5bf6ce98dcfd9". While the Bearer token is properly sanitized as<BEARER_TOKEN>, this SMTP password appears to be from an actual test inbox. Consider masking it (e.g.,"password":"<SMTP_PASSWORD>") for consistency with other credential sanitization in this fixture.lib/mailtrap/inboxes_api.rb (1)
90-92: Consider excludingproject_idfrom request body.The
wrap_requestmethod includes all options in theinboxhash. Forcreate,project_idis already in the URL path, so including it in the body is redundant. While likely harmless, filtering it out would be cleaner.Optional refinement
def wrap_request(options) - { inbox: options } + { inbox: options.except(:project_id) } endexamples/inboxes_api.rb (1)
15-17: Inconsistent naming in update example.The comment correctly says "Update inbox" but the name parameter uses "Test List Updated" which is inconsistent with the Inbox terminology used elsewhere.
📝 Suggested fix
# Update inbox -inboxes.update(inbox.id, name: 'Test List Updated') -# => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated"> +inboxes.update(inbox.id, name: 'Test Inbox Updated') +# => #<struct Mailtrap::Inbox id=1, name="Test Inbox Updated">
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
examples/inboxes_api.rblib/mailtrap/inboxes_api.rbspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/cleans_Inbox_and_returns_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxes_objects.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.ymlspec/mailtrap/inbox_spec.rbspec/mailtrap/inboxes_api_spec.rb
✅ Files skipped from review due to trivial changes (3)
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.yml
🚧 Files skipped from review as they are similar to previous changes (11)
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxes_objects.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/cleans_Inbox_and_returns_Inbox_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.yml
- spec/mailtrap/inbox_spec.rb
- spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.yml
🧰 Additional context used
🧬 Code graph analysis (2)
spec/mailtrap/inboxes_api_spec.rb (1)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)
lib/mailtrap/inboxes_api.rb (2)
lib/mailtrap/base_api.rb (7)
supported_options(27-29)response_class(31-33)base_list(67-70)base_get(46-49)validate_options!(35-40)base_delete(63-65)base_update(57-61)lib/mailtrap/client.rb (2)
post(187-194)patch(201-208)
🪛 ast-grep (0.40.5)
lib/mailtrap/inboxes_api.rb
[warning] 35-36: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes",
wrap_request(options))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (12)
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-77: LGTM!The VCR cassette properly documents the 404 Not Found response for a non-existent inbox. Credentials are appropriately sanitized with
<BEARER_TOKEN>placeholder.lib/mailtrap/inboxes_api.rb (1)
35-38: Static analysis false positive - no hardcoded credentials.The static analysis warning about "hardcoded passphrase" is a false positive. This is simply an API path template string, not a credential.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-77: LGTM!Standard VCR cassette documenting 404 response for reset_credentials on a non-existent inbox. Credentials are properly sanitized.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
1-79: LGTM!Well-structured VCR cassette for testing the 401 Unauthorized response when an incorrect API key is provided. Credentials are properly sanitized with
<BEARER_TOKEN>.examples/inboxes_api.rb (1)
1-33: Overall: Good example coverage of the Inboxes API.The example demonstrates all key operations (create, list, update, get, clean, mark_as_read, reset_credentials, delete) in a logical order. Once the terminology issues are fixed, this will serve as clear documentation for users.
spec/mailtrap/inboxes_api_spec.rb (7)
3-7: LGTM - Well-structured test setup.Good use of environment variable fallbacks for
account_idandapi_key, allowing both local testing with VCR cassettes and live API testing when credentials are provided.
9-27: LGTM - Comprehensive list endpoint tests.The tests properly verify both the happy path (mapping to
Mailtrap::Inboxobjects) and error handling for incorrect API keys.
29-53: LGTM - Get endpoint tests are thorough.Good coverage of both successful retrieval and not-found error scenarios.
55-87: LGTM - Create endpoint tests cover success and validation error cases.
89-135: LGTM - Update endpoint tests are comprehensive.Good coverage including partial updates (hash request with only
name) and not-found scenarios.
137-160: LGTM - Clean endpoint tests are well-structured.
212-235: Delete method should return Inbox object for consistency with similar methods.The
deletemethod currently returns a raw Hash response (viabase_delete), which is why the test correctly uses theincludematcher. However, this is inconsistent with similar methods likeclean,mark_as_read, andreset_credentials, which callhandle_responseto wrap the response in anInboxobject. Consider updatingdeleteto callhandle_responseso it returns anInboxobject and aligns with the established pattern in this API. Also update the docstring from@return nilto@return [Inbox].Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Creates a new inbox | ||
| # @param [Hash] options The parameters to create | ||
| # @option options [String] :name The inbox name | ||
| # @return [Inbox] Created inbox object | ||
| # @!macro api_errors | ||
| # @raise [ArgumentError] If invalid options are provided | ||
| def create(options) | ||
| validate_options!(options, supported_options + [:project_id]) | ||
| response = client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes", | ||
| wrap_request(options)) | ||
| handle_response(response) | ||
| end |
There was a problem hiding this comment.
Missing validation for required project_id parameter.
The create method uses options[:project_id] to construct the API path, but there's no validation ensuring it's present. If project_id is nil or missing, the path becomes malformed (e.g., /api/accounts/123/projects//inboxes), which could result in unexpected errors.
Additionally, the @option documentation only mentions :name but project_id is effectively required.
Proposed fix
# Creates a new inbox
# `@param` [Hash] options The parameters to create
# `@option` options [String] :name The inbox name
+ # `@option` options [Integer] :project_id The project identifier (required)
# `@return` [Inbox] Created inbox object
# @!macro api_errors
# `@raise` [ArgumentError] If invalid options are provided
def create(options)
validate_options!(options, supported_options + [:project_id])
+ raise ArgumentError, 'project_id is required' unless options[:project_id]
response = client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes",
wrap_request(options))
handle_response(response)
end🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 35-36: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes",
wrap_request(options))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🤖 Prompt for AI Agents
In `@lib/mailtrap/inboxes_api.rb` around lines 28 - 39, The create method uses
options[:project_id] in the path but doesn't ensure it's present, so add an
explicit validation that options[:project_id] is provided (raise ArgumentError
with a clear message if missing or nil) before calling client.post; keep the
existing validate_options! call for supported keys but ensure project_id is
treated as required by either adding a presence check or adjusting
validate_options! usage for required keys; also update the YARD docs above
create to include `@option` options [Integer, String] :project_id (marked
required) so callers know project_id is mandatory.
| Email Sandbox (Testing): | ||
|
|
||
| - Project management CRUD – [`projects_api.rb`](examples/projects_api.rb) | ||
| - Inboxes CRUD - [`inboxes_api.rb`](examples/inboxes_api.rb) |
There was a problem hiding this comment.
minor. inconsistent with the previous line
| # => [#<struct Mailtrap::Inbox id=1, name="Test Inbox">] | ||
|
|
||
| # Update inbox | ||
| inboxes.update(inbox.id, name: 'Test List Updated') |
| class InboxesAPI | ||
| include BaseAPI | ||
|
|
||
| self.supported_options = %i[name email_username] |
There was a problem hiding this comment.
you can't specify email_username on inbox creation
Motivation
Cover Inboxes public API with Ruby SDK module.
Changes
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.