Skip to content

Notes returned as valid json instead of inspected string over RPC #21292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sjanusz-r7 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Notes returned as valid json instead of inspected string over RPC #21292

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/msf/core/rpc/v10/rpc_db.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1130,8 +1130,8 @@ def rpc_report_note(xopts)
# * 'time' [Integer] Creation date.
# * 'host' [String] Host address.
# * 'service' [String] Service name or port.
# * 'type' [String] Host type.
# * 'data' [String] Host data.
# * 'type' [String] Note type.
# * 'data' [Hash, String, nil] Note data.
# @example Here's how you would use this from the client:
# # This gives you all the notes.
# rpc.call('db.notes', {})
Expand All @@ -1157,7 +1157,7 @@ def rpc_notes(xopts)
note[:host] = n.host.address if n.host
note[:service] = n.service.name || n.service.port if n.service
note[:type ] = n.ntype.to_s
note[:data] = n.data.inspect
note[:data] = n.data
Comment thread
sjanusz-r7 marked this conversation as resolved.
Comment on lines 1158 to +1160
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This change alters the db.notes RPC response shape (data is no longer always a String). There are existing RPC v10 specs (e.g., for console/core/session), but none covering RPC_Db#rpc_notes; adding a spec that exercises rpc_notes with (1) Hash note data, (2) legacy String note data, and (3) nil/edge cases would help prevent regressions and validate the intended serialization behavior.

Copilot generated this review using guidance from organization custom instructions.
Copy link
Copy Markdown
Contributor

@bwatters-r7 bwatters-r7 Apr 30, 2026

Choose a reason for hiding this comment

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

I second the idea of further rspec tests

ret[:notes] << note
Comment on lines 1159 to 1161
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

rpc_notes now returns n.data directly, which (1) does not implement the requested backward compatibility for legacy notes where data is a plain String (issue asks to convert those to a JSON-like object such as { "data": "..." }), and (2) can return non-JSON/MessagePack-friendly values (e.g., nested Symbols, Times, custom objects) that previously were safely stringified via inspect. Consider normalizing the outgoing value to JSON-safe primitives (deep-stringify keys and coerce unknown values), and wrapping non-Hash data in a predictable structure; also update the YARD docs above this method which still state 'data' [String].

Copilot uses AI. Check for mistakes.
end
ret
Expand Down
Loading