include claude rules#2463
Open
tharindu1st wants to merge 1 commit into
Open
Conversation
tharindu1st
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika and
tharikaGitHub
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughAdded a new set of Claude rule documents covering authentication/authorization, error handling, file access, and post-quantum cryptography guidance for both Go and JavaScript. The updates standardize expected patterns, include clear examples and verification checklists, and expand the repository’s documentation for safer, more consistent implementation across services. WalkthroughChangesThis pull request adds eight documentation-only rule files under Related issues: None specified Related PRs: None specified Suggested labels: Suggested reviewers: Poem: 🚥 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: 11
🧹 Nitpick comments (4)
.claude/rules/js-error-handling.md (1)
104-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmit the correlation ID as
X-Request-ID.Returning only
tracking_idin JSON doesn't match the header-based standard in the directive. Mirror the UUID into the response header, and keep the body field only if consumers truly need it.🤖 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-error-handling.md around lines 104 - 107, The error response in the current handler only returns tracking_id in the JSON body, but it should also emit the same correlation UUID as the X-Request-ID response header. Update the response-building logic around res.status(...).json(...) to set the header before sending the body, and keep tracking_id in the payload only if it is still required by consumers. Use the existing trackingId value in the relevant error-handling path so both header and body stay in sync..claude/rules/js-post-quantum-cryptography.md (2)
13-16: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winScope the X25519 ban to standalone use.
The transition section immediately requires X25519 + ML-KEM-768, so the blanket prohibition contradicts the later guidance. Reword this so only non-hybrid classical key-exchange/signing paths are banned.
🤖 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-post-quantum-cryptography.md around lines 13 - 16, The current guidance in the quantum-safe rules is too broad because it bans X25519 outright, which conflicts with the later hybrid requirement. Update the rules in the post-quantum cryptography doc so `X25519` is only disallowed for standalone classical key-exchange paths, while preserving its use when combined with ML-KEM-768 in the transition guidance; keep the other bans and the `crypto.generateKeyPair`, `crypto.createECDH`, and `crypto.sign` references aligned with this narrower scope.
46-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the randomness rule.
crypto.randomBytesis fine for symmetric keys, but this directive is too absolute for the sample code, which already relies on library-managed randomness (x25519.utils.randomPrivateKey()andml_kem768.encapsulate(...)). Match the wording to 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/js-post-quantum-cryptography.md around lines 46 - 49, Tighten the randomness guidance in the post-quantum cryptography rules so it matches the sample code: keep the requirement that symmetric keys and AES-256-GCM nonces use crypto.randomBytes, but avoid implying every key-generation path must use it. Update the wording in the rules document around the randomness bullets to explicitly allow library-managed randomness used by x25519.utils.randomPrivateKey() and ml_kem768.encapsulate(...), while still forbidding insecure sources like Math.random() and Date.now()..claude/rules/js-authentication-authorization.md (1)
248-252: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocument the org-membership check too.
Sourcing
organizationIdfromreq.useris necessary, butportals/developer-portal/src/middlewares/ensureAuthenticated.js:35-77also rejects org-claim mismatches againstauthorizedOrgs. Call that out here so multi-org sessions keep the second authorization gate.🤖 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-authentication-authorization.md around lines 248 - 252, Update the authentication/authorization guidance to explicitly document the second authorization gate in ensureAuthenticated, where the request user’s organization claim is checked against authorizedOrgs in addition to sourcing organizationId from req.user. Reference the ensureAuthenticated middleware behavior so readers understand that multi-org sessions must pass both JWT-based tenant scoping and org-membership validation before accessing tenant-scoped Sequelize queries.
🤖 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 41-48: The auth failure path in AuthMiddleware is returning a
plain text response via http.Error, which conflicts with the shared sterile JSON
401 contract. Update the AuthMiddleware unauthorized branch to use the same
auth-failure helper used elsewhere in the codebase so the client always receives
the standard JSON payload, while keeping the existing validateToken and
log.Printf error handling intact.
In @.claude/rules/error-handling.md:
- Around line 29-40: The standard payload example in the error-handling guidance
has a malformed code fence, which causes the following note to render inside the
block. Fix the example by removing the stray fence so the JSON snippet under the
“Standard Payload” item is properly closed, and keep the “Note” prose outside
the code block in the surrounding markdown. Refer to the “Standard Payload”
section in the error-handling rules to update the fenced block cleanly.
In @.claude/rules/file-access.md:
- Around line 202-213: The decompression guard in the zip entry reader uses the
wrong bound, because the maxDecompressed calculation in the ratio check is
widened by MaxUploadBytes instead of being capped by it. Update the logic around
maxDecompressed, io.LimitReader, and the decompressed entry size check so the
allowed size is the smaller of the ratio-based limit and cfg.MaxUploadBytes,
preserving the tighter cap for small compressed entries.
- Around line 30-33: The archive-handling guidance here should fail closed
instead of “skip and logged” for malformed entries. Update the rule text to
match the existing behavior in js-file-access and import_artifacts by
instructing callers to reject invalid ZIP/tar entries, stop processing
immediately, and surface an error from the archive validation/extraction flow
rather than partially extracting. Use the archive entry validation language in
this rule to emphasize exact-name allowlisting, path traversal checks, and
erroring on malformed input.
- Around line 186-191: The single-file extraction check is comparing only
basenames, which can pick the wrong archive entry when multiple paths share a
filename. Update the entry selection logic in the extraction flow that uses
cleanedEntry, entryName, and safeJoin so it matches the requested entry by full
normalized relative path instead of filepath.Base(...), and use that same
normalized path when constructing destPath.
- Around line 24-26: The example uses a non-existent Go API, so update the
guidance in the file-access rules to reference os.CreateTemp instead of
io.TempFile. Keep the surrounding advice in the same rule section and make sure
the temporary-file example/wording points to the correct standard-library symbol
so readers can follow a compilable approach.
In @.claude/rules/js-authentication-authorization.md:
- Around line 224-235: The fallback path-check example should be removed because
`req.path` is already decoded by Express and `normalizePath` is doing an unsafe
second `decodeURIComponent()` pass. Update the guidance around `normalizePath`
and the `app.use` auth gate to drop this example entirely, and keep only the
router-scoped allowlist pattern so path handling stays within Express’s decoded
routing behavior.
In @.claude/rules/js-error-handling.md:
- Around line 119-122: The auth-failure logging example in the authentication
handling guidance should stop including the user identifier, since it adds PII
without helping the pattern. Update the logger.warn sample to keep the failure
reason from the auth flow but remove req.body.email from the logged metadata,
using the existing authentication failure example as the place to adjust it.
In @.claude/rules/js-file-access.md:
- Around line 33-40: The archive-entry and decompression-limit failure paths
currently return 422 instead of the documented overflow status. Update the error
handling in the archive limit logic to match the multer limit handler by mapping
these limit/size violations to HTTP 413 Request Entity Too Large with a generic
message, and keep the response body from exposing the configured limit. Use the
existing archive-entry/decompression-limit checks in the JS file-access rules to
locate the affected branches.
- Around line 180-185: The decompression guard in the byte collection logic is
using the more permissive bound instead of the stricter one. Update the
calculation in the block that reads compressedSize, maxDecompressed, and the
surrounding decompression-ratio check to use the smaller of the ratio-based
limit and fileConfig.maxUploadBytes, and keep the absolute limit as the fallback
when compressedSize is unknown. Preserve the existing guard flow around the
decompression bytes collection so the limit is enforced consistently.
- Around line 31-33: The archive entry lookup currently matches by basename,
which can select the wrong file when multiple entries share a filename; update
the exact-file extraction logic to compare the normalized entry path directly in
the relevant archive handling code. Use the existing single-entry extraction
flow and related entry-processing symbols to detect duplicates, fail when more
than one exact path match is found, and keep autodraining non-matching entries.
---
Nitpick comments:
In @.claude/rules/js-authentication-authorization.md:
- Around line 248-252: Update the authentication/authorization guidance to
explicitly document the second authorization gate in ensureAuthenticated, where
the request user’s organization claim is checked against authorizedOrgs in
addition to sourcing organizationId from req.user. Reference the
ensureAuthenticated middleware behavior so readers understand that multi-org
sessions must pass both JWT-based tenant scoping and org-membership validation
before accessing tenant-scoped Sequelize queries.
In @.claude/rules/js-error-handling.md:
- Around line 104-107: The error response in the current handler only returns
tracking_id in the JSON body, but it should also emit the same correlation UUID
as the X-Request-ID response header. Update the response-building logic around
res.status(...).json(...) to set the header before sending the body, and keep
tracking_id in the payload only if it is still required by consumers. Use the
existing trackingId value in the relevant error-handling path so both header and
body stay in sync.
In @.claude/rules/js-post-quantum-cryptography.md:
- Around line 13-16: The current guidance in the quantum-safe rules is too broad
because it bans X25519 outright, which conflicts with the later hybrid
requirement. Update the rules in the post-quantum cryptography doc so `X25519`
is only disallowed for standalone classical key-exchange paths, while preserving
its use when combined with ML-KEM-768 in the transition guidance; keep the other
bans and the `crypto.generateKeyPair`, `crypto.createECDH`, and `crypto.sign`
references aligned with this narrower scope.
- Around line 46-49: Tighten the randomness guidance in the post-quantum
cryptography rules so it matches the sample code: keep the requirement that
symmetric keys and AES-256-GCM nonces use crypto.randomBytes, but avoid implying
every key-generation path must use it. Update the wording in the rules document
around the randomness bullets to explicitly allow library-managed randomness
used by x25519.utils.randomPrivateKey() and ml_kem768.encapsulate(...), while
still forbidding insecure sources like Math.random() and Date.now().
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dd83111-f817-4a6c-bbc8-eb54294b0171
📒 Files selected for processing (8)
.claude/rules/authentication_authorization.md.claude/rules/error-handling.md.claude/rules/file-access.md.claude/rules/js-authentication-authorization.md.claude/rules/js-error-handling.md.claude/rules/js-file-access.md.claude/rules/js-post-quantum-cryptography.md.claude/rules/post-quantum-cryptography.md
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
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
1 participant
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 claude rules