Skip to content

Add C-CALLER-CONTROL exceptions for crypto and internal APIs#30

Closed
DanGould wants to merge 1 commit intomasterfrom
coderabbit-refine-exceptions
Closed

Add C-CALLER-CONTROL exceptions for crypto and internal APIs#30
DanGould wants to merge 1 commit intomasterfrom
coderabbit-refine-exceptions

Conversation

@DanGould
Copy link
Copy Markdown
Owner

@DanGould DanGould commented Mar 2, 2026

Summary

Add exceptions to the C-CALLER-CONTROL rule so CodeRabbit doesn't
flag legitimate &[u8] usage in crypto/HPKE functions where internal
buffering requires a copy regardless of ownership.

Approach

Added an EXCEPTIONS block to both path instructions that excludes:

  • Functions where clone/to_vec is required for mutation, padding, or
    encryption buffering
  • pub(crate) and private functions (not public API)
  • Idiomatic borrow-through patterns (e.g. &[u8] passed to crypto
    primitives that also take &[u8])

Open questions

None.

Pull Request Checklist
  • I have disclosed my use of AI in the body of this PR.
  • I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

Disclosure: co-authored by Claude

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

This change modifies the .coderabbit.yaml configuration file to add EXCEPTIONS blocks for C-CALLER-CONTROL checks in YAML path instruction sections. The exceptions define conditions where specific patterns should not be flagged, including: cases where cloning or to_vec operations are necessary due to data mutation, padding, or extension requirements; functions with pub(crate) or private visibility; and idiomatic borrowing patterns such as passing \&[u8] to primitive functions. The existing guidance remains otherwise unchanged. This is a configuration-only update with no functional code modifications.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch coderabbit-refine-exceptions

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.coderabbit.yaml (1)

77-83: Consider aligning wording with the first path_instructions block for consistency.

This EXCEPTIONS block is functionally equivalent to lines 39-50 but omits the detailed rationale (e.g., "callers should not have to pre-allocate a Vec..."). While brevity is fine for the CLI path, having identical exception text in both blocks would reduce maintenance burden and avoid potential drift if the rules are updated independently later.

📝 Suggested alignment
         EXCEPTIONS — do NOT flag C-CALLER-CONTROL when:
         - The clone/to_vec is required because the algorithm mutates,
-          pads, or extends the data.
-        - The function is pub(crate) or private.
+          pads, or extends the data (e.g. encryption, HPKE, padding).
+          Borrowing &[u8] is correct here; callers should not have to
+          pre-allocate a Vec just to pass owned data that will be
+          copied into an internal buffer anyway.
+        - The function is pub(crate) or private, not part of the
+          public API.
         - The parameter is used in a context where borrowing is
-          idiomatic (e.g. &[u8] passed to a crypto primitive).
+          idiomatic (e.g. &[u8] passed directly to a crypto primitive
+          that also takes &[u8]).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9a1acc and cbf372d.

📒 Files selected for processing (1)
  • .coderabbit.yaml

@DanGould
Copy link
Copy Markdown
Owner Author

DanGould commented Mar 4, 2026

I loathe these verbose imprecise exceptions. This aint it. I didn't make these exceptions upstream in payjoin#1373

@DanGould DanGould closed this Mar 4, 2026
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.

1 participant