Conversation
📝 WalkthroughWalkthroughAdds a new SECURITY.md pointing reports to security@openzeppelin.com, inserts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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🧪 Generate unit tests (beta)
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 |
luiz-lvj
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@SECURITY.md`:
- Around line 1-3: Update SECURITY.md to use the same vulnerability contact as
the contracts by replacing the email string "security@openzeppelin.com" with
"security@openzeppelin.org", and fix the broken Markdown link by either
converting "[Immunefi]" to an inline link like "Immunefi (https://immunefi.com)"
or adding a reference definition for [Immunefi] with the correct URL (e.g.,
https://immunefi.com) so the link renders and the contact addresses are
consistent.
SECURITY.md
Outdated
| # Security Policy | ||
|
|
||
| Security vulnerabilities should be disclosed to the project maintainers through [Immunefi], or alternatively by email to security@openzeppelin.com. |
There was a problem hiding this comment.
Email domain mismatch and broken Markdown link.
-
The email here is
security@openzeppelin.com(.com), but every Solidity contract in this PR usessecurity@openzeppelin.org(.org). These should be consistent to avoid misdirected vulnerability reports. -
[Immunefi]is a reference-style Markdown link with no corresponding URL definition, so it will render as plain text rather than a clickable link.
Proposed fix
# Security Policy
-Security vulnerabilities should be disclosed to the project maintainers through [Immunefi], or alternatively by email to security@openzeppelin.com.
+Security vulnerabilities should be disclosed to the project maintainers through [Immunefi](https://immunefi.com), or alternatively by email to security@openzeppelin.org.📝 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.
| # Security Policy | |
| Security vulnerabilities should be disclosed to the project maintainers through [Immunefi], or alternatively by email to security@openzeppelin.com. | |
| # Security Policy | |
| Security vulnerabilities should be disclosed to the project maintainers through [Immunefi](https://immunefi.com), or alternatively by email to security@openzeppelin.org. |
🤖 Prompt for AI Agents
In `@SECURITY.md` around lines 1 - 3, Update SECURITY.md to use the same
vulnerability contact as the contracts by replacing the email string
"security@openzeppelin.com" with "security@openzeppelin.org", and fix the broken
Markdown link by either converting "[Immunefi]" to an inline link like "Immunefi
(https://immunefi.com)" or adding a reference definition for [Immunefi] with the
correct URL (e.g., https://immunefi.com) so the link renders and the contact
addresses are consistent.
SECURITY.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| # Security Policy | |||
|
|
|||
| Security vulnerabilities should be disclosed to the project maintainers through [Immunefi], or alternatively by email to security@openzeppelin.com. | |||
There was a problem hiding this comment.
this codebase is not in inmunefi, so please remove it and just keep the email
| /// a sliding window of block hashes without requiring contiguous block numbers. | ||
| /// @dev Concrete implementations should override `receiveHashes` to add chain-specific access control. | ||
| /// @notice Inspired by: https://github.com/OffchainLabs/block-hash-pusher/blob/main/contracts/Buffer.sol | ||
| /// @custom:security-contact security@openzeppelin.org |
There was a problem hiding this comment.
isn't the email @openzeppelin.COM ?
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/contracts/block-hash-pusher/zksync/ZkSyncPusher.sol (1)
9-9: Consider using a structured NatSpec tag for the source reference.The
/// Source:line is an untagged///comment. If no tags are used, the Solidity compiler will interpret a///comment the same way as if it were tagged with@notice, which means this line is semantically conflated with the interface-level notice rather than being categorized as a cross-reference. Using///@devSource:or///@Custom:sourcewould give the attribution a distinct NatSpec category in the compiled devdoc output.♻️ Suggested improvement
-/// Source: https://github.com/matter-labs/era-contracts/blob/33fbd0d832d15da150dcc7ec8032660980caa692/l1-contracts/contracts/state-transition/chain-interfaces/IMailboxImpl.sol#L88 +/// `@dev` Source: https://github.com/matter-labs/era-contracts/blob/33fbd0d832d15da150dcc7ec8032660980caa692/l1-contracts/contracts/state-transition/chain-interfaces/IMailboxImpl.sol#L88🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/contracts/block-hash-pusher/zksync/ZkSyncPusher.sol` at line 9, Replace the untagged triple-slash comment that points to the upstream source with a NatSpec-tagged comment so it is categorized properly in devdoc; in ZkSyncPusher.sol (the top-of-file comment referencing the Matter Labs IMailboxImpl URL) change the line from a plain "/// Source: ..." to a tagged form such as "/// `@dev` Source: ..." or "/// `@custom`:source: ..." so the reference is kept but is not treated as `@notice` in the compiled docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/contracts/block-hash-pusher/zksync/ZkSyncPusher.sol`:
- Line 9: Replace the untagged triple-slash comment that points to the upstream
source with a NatSpec-tagged comment so it is categorized properly in devdoc; in
ZkSyncPusher.sol (the top-of-file comment referencing the Matter Labs
IMailboxImpl URL) change the line from a plain "/// Source: ..." to a tagged
form such as "/// `@dev` Source: ..." or "/// `@custom`:source: ..." so the
reference is kept but is not treated as `@notice` in the compiled docs.
2 participants
Providing a specific security contact (such as an email address or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice is quite beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for their maintainers to contact the appropriate person about the problem and provide mitigation instructions.
Throughout the codebase, multiple instances of contracts not having a security contact were identified.
This PR adds a NatSpec comment containing a security contact above each contract definition. It also adds a
SECURITY.mdfileSummary by CodeRabbit