include other claude rules#2538
Merged
Merged
Conversation
tharindu1st
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub and
thivindu
as code owners
July 8, 2026 06:08
Contributor
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds and extends ChangesSecurity rules documentation
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
.claude/rules/js-output-encoding-xss.md (1)
27-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the CSP example to the portal's current CDN-backed assets.
A literal
script-src 'self'example conflicts with the existing CDN script whitelist inportals/developer-portal/src/utils/util.js:700-720, so copying it as-is would break pages that still load those assets. Either call out that the example assumes self-hosted scripts, or list the required hashes/origins for the current model.As shown in
portals/developer-portal/src/utils/util.js:700-720, the current asset-loading contract still depends on CDN scripts.Also applies to: 136-149
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/js-output-encoding-xss.md around lines 27 - 29, Scope the CSP guidance in the JS output encoding doc so it matches the portal’s current CDN-backed asset model rather than implying a blanket script-src 'self' requirement. Update the CSP example in the affected section to either explicitly say it applies only to self-hosted scripts or to include the needed CDN origins/hashes already used by the asset-loading logic in util.js, and make sure the wording stays consistent with the related guidance elsewhere in the document..claude/rules/js-xxe-xml-processing.md (1)
22-24: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDon’t present
stopNodesas a depth limit.This reads like
fast-xml-parserhas a real nesting-cap option here, but that may not be what the API actually provides. Please verify the parser docs and reword this to a genuine depth cap or the manual depth-walk / worker-timeout fallback unless the library confirms a real limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/js-xxe-xml-processing.md around lines 22 - 24, The XML parsing guidance currently treats fast-xml-parser stopNodes as if it were a nesting-depth limit, which may be inaccurate. Reword the Depth Ceiling section in js-xxe-xml-processing.md to reference only a real parser depth-cap if the library docs confirm one; otherwise instruct implementers to add a manual depth-tracking validation pass before parsing or use a worker-thread timeout fallback. Ensure the guidance stays aligned with the symbols and concepts in the existing file, especially the parser depth and parse-timeout recommendations..claude/rules/xxe-xml-processing.md (1)
85-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShow a config-backed ceiling in the example.
maxDocBytesis hardcoded here even though the rule says the XML byte ceiling must come from config. A reader can copy this sample verbatim and silently violate the policy. Please inject the configured limit here instead of baking5 << 20into the example.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/xxe-xml-processing.md around lines 85 - 89, The example for the XML parser settings hardcodes the document byte ceiling instead of showing the config-backed value required by the rule. Update the snippet around the maxDocBytes constant to reference the configured limit used by the real call sites, keeping maxElemDepth and parseTimeout as-is, so readers copy an example that matches the policy and symbols like maxDocBytes remain clearly tied to configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/authentication_authorization.md:
- Around line 632-643: The safeRedirectTarget helper currently accepts host-less
opaque URLs, which can still produce unsafe redirect locations. Update
safeRedirectTarget to only allow path-only targets by requiring an empty scheme
and opaque component in addition to no host and no leading "//", and only return
success when the parsed value starts with "/". Keep the validation and return
logic inside safeRedirectTarget consistent so malformed or non-path URLs are
rejected before being used as a redirect target.
In @.claude/rules/js-authentication-authorization.md:
- Around line 603-614: The authMiddleware example calls jwtVerify(token, ...)
before token is defined, so make the snippet copy-pastable by extracting the
bearer token from req.headers.authorization first and binding it to token before
verification. Update authMiddleware to parse the Authorization header, validate
the Bearer format, then pass the extracted token into jwtVerify while keeping
the existing User.findByPk and req.user assignment logic unchanged.
In @.claude/rules/js-file-access.md:
- Around line 254-262: The icon upload handler reads req.file.buffer before
checking whether a file was actually uploaded, so the route can throw when
upload.single('icon') reaches it without a file. Update the /apis/:id/icon
handler to guard req.file at the start of the async callback before calling
validateUploadContent or ApiIcon.create, and return a generic 400/422 response
when missing; use the existing route handler and req.file reference to place the
fix.
In @.claude/rules/js-ssrf-prevention.md:
- Around line 130-146: The import in the spec URL handler is pulling
assertAllowedScheme from the wrong module, so the example will break when
reused. Update the import in the /api/specs/import-from-url handler to get
assertAllowedScheme from ssrfGuard.js and keep safeHttpClient from
safeHttpClient.js, so the scheme check and the fetch both reference the correct
symbols.
- Line 19: The reserved-range denylist in the SSRF prevention rules contains an
incorrect loopback CIDR example, so update the “Deny Reserved Ranges” entry to
use the proper loopback range instead of 127.0.0.0/0. Fix the list in the
js-ssrf-prevention guidance so the IPv4 loopback CIDR is accurate and the
examples remain safe and correct alongside the other reserved ranges.
- Around line 91-124: The scheme check in assertAllowedScheme currently allows
plain http: by default, which should be opt-in instead. Update
assertAllowedScheme so http: is rejected unless an explicit allowHttp flag is
passed, while still allowing https: and preserving the existing statusCode
behavior for disallowed schemes. Keep the change localized to the URL parsing
logic in assertAllowedScheme and its callers, including safeHttpClient if
needed.
In @.claude/rules/ssrf-prevention.md:
- Line 19: The SSRF deny-list example in the reserved ranges guidance uses an
incorrect loopback CIDR, which would match every IPv4 address instead of just
loopback. Update the reserved-range list in the SSRF prevention rule to use the
correct loopback block, and keep the rest of the deny-list examples in line with
the intended semantics for private, loopback, link-local, and metadata
destinations.
---
Nitpick comments:
In @.claude/rules/js-output-encoding-xss.md:
- Around line 27-29: Scope the CSP guidance in the JS output encoding doc so it
matches the portal’s current CDN-backed asset model rather than implying a
blanket script-src 'self' requirement. Update the CSP example in the affected
section to either explicitly say it applies only to self-hosted scripts or to
include the needed CDN origins/hashes already used by the asset-loading logic in
util.js, and make sure the wording stays consistent with the related guidance
elsewhere in the document.
In @.claude/rules/js-xxe-xml-processing.md:
- Around line 22-24: The XML parsing guidance currently treats fast-xml-parser
stopNodes as if it were a nesting-depth limit, which may be inaccurate. Reword
the Depth Ceiling section in js-xxe-xml-processing.md to reference only a real
parser depth-cap if the library docs confirm one; otherwise instruct
implementers to add a manual depth-tracking validation pass before parsing or
use a worker-thread timeout fallback. Ensure the guidance stays aligned with the
symbols and concepts in the existing file, especially the parser depth and
parse-timeout recommendations.
In @.claude/rules/xxe-xml-processing.md:
- Around line 85-89: The example for the XML parser settings hardcodes the
document byte ceiling instead of showing the config-backed value required by the
rule. Update the snippet around the maxDocBytes constant to reference the
configured limit used by the real call sites, keeping maxElemDepth and
parseTimeout as-is, so readers copy an example that matches the policy and
symbols like maxDocBytes remain clearly tied to configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65486bdd-7a40-4e75-a773-1529a8f54940
📒 Files selected for processing (9)
.claude/rules/authentication_authorization.md.claude/rules/file-access.md.claude/rules/js-authentication-authorization.md.claude/rules/js-file-access.md.claude/rules/js-output-encoding-xss.md.claude/rules/js-ssrf-prevention.md.claude/rules/js-xxe-xml-processing.md.claude/rules/ssrf-prevention.md.claude/rules/xxe-xml-processing.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/authentication_authorization.md:
- Around line 549-580: Make the credential invalidation transition atomic and
concurrency-safe: update LockAccountHandler and UpdateUserRoleHandler so the
account/role change and token revocation, or a durable revocation marker, commit
within the same transaction and cannot leave old credentials valid on failure or
races. In .claude/rules/js-authentication-authorization.md lines 588-590,
replace user.tokenVersion + 1 with a row-locked or atomic increment so
concurrent updates cannot reuse a version.
In @.claude/rules/js-authentication-authorization.md:
- Around line 661-672: Update safeRedirectTarget to validate that raw is a
non-empty string before constructing the URL or calling startsWith; return '/'
for arrays, objects, empty values, and other non-string inputs, while preserving
the existing relative-target handling for valid strings.
In @.claude/rules/js-output-encoding-xss.md:
- Around line 153-157: Update the SVG upload verification checklist to remove
any implication that X-Content-Type-Options: nosniff alone is sufficient.
Require SVG-aware sanitization together with Content-Disposition: attachment or
serving from a separate cookie-less origin, and retain nosniff only as
additional defense in depth.
- Around line 114-118: Update the POST /apis/:id/icon handler to stop relying on
req.file.mimetype for content validation or response metadata. Sniff the
uploaded bytes server-side, validate that the detected format matches the
declared type, reject mismatches and unsupported formats, and sanitize SVG
content before persisting it. Persist only the validated MIME type alongside the
validated data for later Content-Type responses.
In @.claude/rules/js-ssrf-prevention.md:
- Around line 76-79: Add the IPv6 unique-local CIDR fc00::/7 to DENIED_CIDRS in
.claude/rules/js-ssrf-prevention.md (lines 76-79) and deniedCIDRs in
.claude/rules/ssrf-prevention.md (lines 93-96), and extend both SSRF rule test
guidance with IPv6 unique-local address variants.
In @.claude/rules/ssrf-prevention.md:
- Around line 142-154: The SafeHTTPClient CheckRedirect callback must not follow
unvalidated redirect targets. Update CheckRedirect to disable redirects with
http.ErrUseLastResponse, or validate each req.URL against the original host
allowlist and HTTPS policy before returning nil; preserve the existing redirect
limit only if redirects remain enabled.
In @.claude/rules/xxe-xml-processing.md:
- Around line 112-125: Update the ParseXML flow around decodeWithDepthLimit so a
timeout cancels or bounds the underlying XML parsing work instead of merely
returning while the worker continues. Use a genuinely cooperative cancellable
parser, or a bounded worker mechanism that prevents timed-out parses from
accumulating indefinitely, while preserving the existing timeout error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c982082-0a6e-45e5-8316-200177ca11c7
📒 Files selected for processing (9)
.claude/rules/authentication_authorization.md.claude/rules/file-access.md.claude/rules/js-authentication-authorization.md.claude/rules/js-file-access.md.claude/rules/js-output-encoding-xss.md.claude/rules/js-ssrf-prevention.md.claude/rules/js-xxe-xml-processing.md.claude/rules/ssrf-prevention.md.claude/rules/xxe-xml-processing.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude/rules/js-xxe-xml-processing.md
- .claude/rules/js-file-access.md
- .claude/rules/file-access.md
Krishanx92
previously approved these changes
Jul 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
.claude/rules/authentication_authorization.md (1)
576-583: 🔒 Security & Privacy | 🟠 MajorKeep revocation failure handling consistent with the rule.
The example treats
RevokeAllForUseras best-effort and still returns204, while the checklist requires revocation failure to fail the operation. Either make the durable marker authoritative for every credential type, or make session/token cleanup durable and fail the handler when it cannot complete.Also applies to: 617-620
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/authentication_authorization.md around lines 576 - 583, Update the handler around RevokeAllForUser so revocation failures do not return http.StatusNoContent: make session/token cleanup durable and return the rule’s required failure response when it cannot complete, or ensure the durable token_version marker is authoritative for every credential type before retaining best-effort cleanup. Apply the same behavior to the corresponding revocation path noted at lines 617-620.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/authentication_authorization.md:
- Around line 585-600: Update UpdateUserRoleHandler to enforce GO-AUTH-007
caller-scope authorization before beginning the role-changing transaction, then
validate newRole against the permitted role allowlist and transition rules
before executing the accounts update. Reject unauthorized callers and invalid
role transitions without modifying the database.
In @.claude/rules/js-output-encoding-xss.md:
- Around line 124-140: Guard the upload handler before accessing req.file.buffer
by returning a controlled 4xx response when req.file is absent. In the icon
retrieval flow, validate the icon record before dereferencing icon.mimeType and
return 404 when no icon exists; preserve the existing response behavior for
valid records.
In @.claude/rules/js-ssrf-prevention.md:
- Around line 76-80: Extend the reserved-range denylists to reject unspecified
destinations in both SSRF implementations: add the unspecified IPv4 and IPv6
CIDRs to DENIED_CIDRS in .claude/rules/js-ssrf-prevention.md at lines 76-80 and
to deniedCIDRs in .claude/rules/ssrf-prevention.md at lines 93-97.
- Around line 82-87: Update isDenied to convert IPv4-mapped IPv6 addresses to
their IPv4 representation before comparing address kinds and matching
DENIED_CIDRS, while preserving normal IPv6 handling. Add regression tests
covering mapped loopback and private addresses to verify they are denied by the
corresponding IPv4 ranges.
In @.claude/rules/xxe-xml-processing.md:
- Around line 95-113: Update ParseXML so the parseTimeout context is created
before io.ReadAll, ensuring the deadline covers input reading as well as
parsing. Replace the current raw reader flow with the established context-aware
or bounded reader mechanism, and preserve the existing maxDocBytes limit and
read-error handling.
---
Duplicate comments:
In @.claude/rules/authentication_authorization.md:
- Around line 576-583: Update the handler around RevokeAllForUser so revocation
failures do not return http.StatusNoContent: make session/token cleanup durable
and return the rule’s required failure response when it cannot complete, or
ensure the durable token_version marker is authoritative for every credential
type before retaining best-effort cleanup. Apply the same behavior to the
corresponding revocation path noted at lines 617-620.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4a5dddc-c121-4372-9cfd-e0190d50de83
📒 Files selected for processing (9)
.claude/rules/authentication_authorization.md.claude/rules/file-access.md.claude/rules/js-authentication-authorization.md.claude/rules/js-file-access.md.claude/rules/js-output-encoding-xss.md.claude/rules/js-ssrf-prevention.md.claude/rules/js-xxe-xml-processing.md.claude/rules/ssrf-prevention.md.claude/rules/xxe-xml-processing.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude/rules/file-access.md
- .claude/rules/js-file-access.md
- .claude/rules/js-xxe-xml-processing.md
tharindu1st
force-pushed
the
rules
branch
2 times, most recently
from
July 21, 2026 17:57
529ce0d to
dbf30ee
Compare
renuka-fernando
previously approved these changes
Jul 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.claude/rules/authentication_authorization.md (1)
576-581: 🔒 Security & Privacy | 🟠 MajorDo not treat session revocation as best effort.
The handler logs
RevokeAllForUserfailure and still returns success.token_versiononly invalidates paths that check it; active session records can remain usable. Make revocation durable/atomic with the state change, or fail the operation when cleanup fails. This repeats the previously reported revocation gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/rules/authentication_authorization.md around lines 576 - 581, The handler must not continue successfully when RevokeAllForUser fails. Update the flow around tokenStore.RevokeAllForUser to make revocation atomic with the token_version state change where supported, or propagate the error and return a failure response, ensuring the state change is rolled back or not committed when cleanup cannot complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/authentication_authorization.md:
- Around line 690-707: Reject userinfo in both safe redirect implementations: in
safeRedirectTarget, require u.User == nil before returning an allowlisted
absolute HTTPS redirect; in .claude/rules/js-authentication-authorization.md
lines 667-682, reject parsed URLs with non-empty username or password before
accepting the absolute redirect. Preserve existing relative-path and host
allowlist checks.
In @.claude/rules/js-ssrf-prevention.md:
- Around line 99-105: Update guardedLookup to handle dns.lookup callbacks with
options.all enabled: when the result is an array of address/family objects,
reject if any address is denied and otherwise return the full array with the
existing callback contract. Preserve the current single-address validation and
callback behavior when options.all is false.
In @.claude/rules/xxe-xml-processing.md:
- Around line 102-149: The timeout branches in ParseXML must not return while
the read or decode goroutines can remain blocked or continue processing. Update
the readResult/readCh and errCh flows around io.ReadAll and decodeWithDepthLimit
to use cooperative cancellation and ensure each worker exits before ParseXML
returns, or isolate them with a bounded mechanism that guarantees cleanup.
Preserve the existing size checks, DOCTYPE rejection, and timeout error
behavior.
---
Duplicate comments:
In @.claude/rules/authentication_authorization.md:
- Around line 576-581: The handler must not continue successfully when
RevokeAllForUser fails. Update the flow around tokenStore.RevokeAllForUser to
make revocation atomic with the token_version state change where supported, or
propagate the error and return a failure response, ensuring the state change is
rolled back or not committed when cleanup cannot complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6e09bd9-732d-451d-8bbf-1cb611b6e454
📒 Files selected for processing (9)
.claude/rules/authentication_authorization.md.claude/rules/file-access.md.claude/rules/js-authentication-authorization.md.claude/rules/js-file-access.md.claude/rules/js-output-encoding-xss.md.claude/rules/js-ssrf-prevention.md.claude/rules/js-xxe-xml-processing.md.claude/rules/ssrf-prevention.md.claude/rules/xxe-xml-processing.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude/rules/js-xxe-xml-processing.md
- .claude/rules/file-access.md
- .claude/rules/js-file-access.md
renuka-fernando
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
include other claude rules