Matin/ Added security NCLC review github workflow

[matin-deriv]

Changes:

• Added security NCLC review github workflow
• Added Claude.md file to the project
• Removed unnecessary tests
• ShiftAI setup
• Claude.md workflow for code reviews

Type of change

• Bug fix
• New feature
• Update feature
• Refactor code
• Translation to code
• Translation to crowdin
• Script configuration
• Improve performance
• Style only
• Dependency update
• Documentation update
• Release

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details
actions/actions/checkout	4.*.*	🟢 6.5	Details Check Score Reason Dangerous-Workflow 🟢 10 no dangerous workflow patterns detected Maintained 🟢 5 6 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 5 Code-Review 🟢 10 all changesets reviewed Binary-Artifacts 🟢 10 no binaries found in the repo CII-Best-Practices ⚠️ 0 no effort to earn an OpenSSF best practices badge detected Token-Permissions ⚠️ 0 detected GitHub workflow tokens with excessive permissions Fuzzing ⚠️ 0 project is not fuzzed License 🟢 10 license file detected Packaging ⚠️ -1 packaging workflow not detected Signed-Releases ⚠️ -1 no releases found Pinned-Dependencies 🟢 3 dependency not pinned by hash detected -- score normalized to 3 Security-Policy 🟢 9 security policy file detected Vulnerabilities 🟢 9 1 existing vulnerabilities detected Branch-Protection 🟢 5 branch protection is not maximal on development and all release branches SAST 🟢 8 SAST tool detected but not run on all commits
actions/anthropics/claude-code-security-review	68982a6bf10d545e94dd0390af08306d94ef684c	Unknown	Unknown
actions/slackapi/slack-github-action	6c661ce58804a1a20f6dc5fbee7f0381b469e001	🟢 6.3	Details Check Score Reason Binary-Artifacts 🟢 10 no binaries found in the repo Packaging ⚠️ -1 packaging workflow not detected Code-Review 🟢 10 all changesets reviewed Maintained 🟢 10 24 commit(s) and 3 issue activity found in the last 90 days -- score normalized to 10 Token-Permissions 🟢 8 detected GitHub workflow tokens with excessive permissions Dangerous-Workflow ⚠️ 0 dangerous workflow patterns detected Pinned-Dependencies 🟢 8 dependency not pinned by hash detected -- score normalized to 8 CII-Best-Practices ⚠️ 0 no effort to earn an OpenSSF best practices badge detected Security-Policy ⚠️ 0 security policy file not detected License 🟢 10 license file detected Fuzzing ⚠️ 0 project is not fuzzed Signed-Releases ⚠️ -1 no releases found Branch-Protection 🟢 5 branch protection is not maximal on development and all release branches Vulnerabilities 🟢 9 1 existing vulnerabilities detected SAST 🟢 10 SAST tool is run on all commits

Scanned Manifest Files

.github/workflows/security-nclc-review.yml

• actions/checkout@4.*.*
• anthropics/claude-code-security-review@68982a6
• slackapi/slack-github-action@6c661ce

package-lock.json

• @deriv-com/shiftai-cli@1.0.12
• @inquirer/external-editor@1.0.3
• @isaacs/cliui@8.0.2
• @kwsites/file-exists@1.1.1
• @kwsites/promise-deferred@1.1.1
• @octokit/auth-token@4.0.0
• @octokit/core@5.2.2
• @octokit/endpoint@9.0.6
• @octokit/graphql@7.1.1
• @octokit/openapi-types@24.2.0
• @octokit/plugin-paginate-rest@11.4.4-cjs.2
• @octokit/plugin-request-log@4.0.1
• @octokit/plugin-rest-endpoint-methods@13.3.2-cjs.1
• @octokit/request@8.4.1
• @octokit/request-error@5.1.1
• @octokit/rest@20.1.2
• @octokit/types@13.10.0
• @pkgjs/parseargs@0.11.0
• @types/tinycolor2@1.4.6
• ansi-align@3.0.1
• ansi-escapes@4.3.2
• ansi-regex@6.2.2
• ansi-styles@6.2.3
• arch@2.2.0
• base64-js@1.5.1
• before-after-hook@2.2.3
• bl@4.1.0
• boxen@5.1.2
• buffer@5.7.1
• chardet@2.1.1
• cli-boxes@2.2.1
• cli-cursor@3.1.0
• cli-spinners@2.9.2
• cli-width@3.0.0
• clipboardy@2.3.0
• clone@1.0.4
• commander@9.5.0
• commander@14.0.2
• cross-spawn@6.0.6
• debug@4.4.3
• defaults@1.0.4
• deprecation@2.3.1
• dotenv@16.6.1
• eastasianwidth@0.2.0
• emoji-regex@9.2.2
• execa@1.0.0
• figlet@1.9.4
• foreground-child@3.3.1
• fs-extra@11.3.3
• get-stream@4.1.0
• glob@10.5.0
• gradient-string@2.0.2
• husky@9.1.7
• iconv-lite@0.7.1
• ieee754@1.2.1
• inquirer@8.2.7
• is-interactive@1.0.0
• is-stream@1.1.0
• jackspeak@3.4.3
• jsonfile@6.2.0
• minimatch@9.0.5
• minipass@7.1.2
• mute-stream@0.0.8
• nice-try@1.0.5
• node-fetch@2.7.0
• onetime@5.1.2
• ora@5.4.1
• p-finally@1.0.0
• package-json-from-dist@1.0.1
• path-scurry@1.11.1
• progress@2.0.3
• pump@3.0.3
• readable-stream@3.6.2
• restore-cursor@3.1.0
• run-async@2.4.1
• shebang-command@1.2.0
• shebang-regex@1.0.0
• signal-exit@3.0.7
• simple-git@3.30.0
• string-width@5.1.2
• string-width-cjs@4.2.3
• strip-ansi@7.1.2
• strip-ansi-cjs@6.0.1
• strip-eof@1.0.0
• through@2.3.8
• tinycolor2@1.6.0
• tinygradient@1.1.5
• tr46@0.0.3
• type-fest@0.21.3
• universal-user-agent@6.0.1
• universalify@2.0.1
• wcwidth@1.0.1
• webidl-conversions@3.0.1
• whatwg-url@5.0.0
• widest-line@3.1.0
• wrap-ansi@8.1.0
• wrap-ansi@6.2.0
• wrap-ansi-cjs@7.0.0

[github-actions]

Claude finished @matin-deriv's task in 2m 2s —— View job

---

Code Review Complete ✅

PR #1209: Security NCLC review workflow and Claude.md setup

I've completed a comprehensive security-focused review of this PR. Here are my findings organized by priority:

---

🔴 CRITICAL Issues

1. Secret Exposure Risk in Workflow

Location: .github/workflows/claude.yml:32, 55
Issue: Using PERSONAL_ACCESS_TOKEN and ANTHROPIC_API_KEY secrets
Risk: While the secrets themselves are properly referenced, the workflow runs on PRs from forks (line 38-39 checks out github.event.pull_request.head.repo.full_name). This could allow malicious PRs to trigger workflows, though the verify_user_in_organization step mitigates this.
Recommendation: ✅ The verification step at line 28-32 is good defense-in-depth. However, consider adding explicit conditions to prevent workflow runs on forks:

jobs:
    claude-assistant:
        if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository

2. Workflow Permission Scope

Location: .github/workflows/claude.yml:15-20
Issue: id-token: write permission granted
Risk: This permission allows workflows to request OIDC tokens, which could be misused if the workflow is compromised
Assessment: Given that this is for Claude Code Action which may need authentication, this might be intentional. However, verify if this permission is actually required by the action.
Recommendation: If not needed, remove id-token: write permission.

---

:large_orange_circle: MEDIUM Priority Issues

3. Husky Hook Error Handling

Location: .husky/pre-commit:8
Issue: Pre-commit hook preserves exit code from ShiftAI CLI
Risk: If the ShiftAI CLI fails or has a bug, it will block all commits
Current Code:

npx @deriv-com/shiftai-cli hook pre-commit
exit $?

Recommendation: Add error handling to prevent blocking commits on tool failure:

npx @deriv-com/shiftai-cli hook pre-commit || {
    echo "Warning: ShiftAI pre-commit hook failed, but allowing commit to proceed"
    exit 0
}

4. Package Dependency Security

Location: package.json:35
Issue: New dependency @deriv-com/shiftai-cli@^1.0.12 added
Risk: This is a private/company package. Ensure it's from a trusted source and regularly maintained
Recommendation:

• Verify the package comes from an official Deriv repository
• Consider pinning to exact version instead of using ^ for security-critical tools
• Review the package's dependencies in package-lock.json for known vulnerabilities

5. Workflow Timeout Configuration

Location: .github/workflows/claude.yml:52
Issue: 60-minute timeout for Claude Code Action
Risk: Long-running workflows increase exposure window for potential issues and consume excessive resources
Recommendation: Monitor actual execution times and reduce timeout to a more reasonable value (e.g., 15-30 minutes) if possible.

6. Missing Rate Limiting Context

Location: Both workflow files
Issue: No visible rate limiting or concurrency controls beyond basic cancel-in-progress
Risk: Multiple PRs or rapid updates could exhaust API quotas or create resource contention
Current: .github/workflows/claude.yml has per-PR concurrency control (good)
Recommendation: Consider adding repository-level rate limiting if API costs become a concern.

---

🔵 LOW Priority / Observations

7. Test File Modifications

Location: src/javascript/_common/base/__tests__/client_base.js, src/javascript/app/pages/user/account/__tests__/portfolio.js
Changes: Removed unnecessary API calls in tests, replacing with mocked data
Assessment: ✅ This is a positive change - reduces test dependencies and makes tests more reliable
Security Impact: None - these are test improvements

8. .gitignore Additions

Location: .gitignore:14-22
Changes: Added .env files and AI tool configuration files to gitignore
Assessment: ✅ Excellent security practice - prevents accidental commit of secrets and sensitive configuration
Recommendation: None needed - this follows best practices

9. CLAUDE.md Documentation Quality

Location: CLAUDE.md
Assessment: ✅ Comprehensive, well-structured documentation with:

• Clear architecture overview
• Security guidelines (no hardcoded secrets, validation patterns)
• Naming conventions and code patterns
• Development workflows
  Security Impact: Positive - provides AI assistants with security-aware coding guidelines
  Minor Suggestion: Consider adding a section on security testing procedures

10. Security Workflow Custom Instructions

Location: .github/workflows/security-nclc-review.yml:31-176
Assessment: ✅ Exceptionally thorough security review checklist covering:

• XSS, SQL injection, RCE, path traversal
• React-specific security patterns
• Cryptography and secrets management
• OWASP/CWE references
  Recommendation: This is comprehensive and well-designed. No changes needed.

11. Slack Notification on Failure

Location: .github/workflows/security-nclc-review.yml:184-207
Assessment: ✅ Good practice - ensures security scan failures get visibility
Security Impact: Positive - enables rapid response to security issues
Minor Note: The payload exposes PR creator and committer usernames, which is acceptable for internal notifications

---

✅ Positive Security Practices Observed

1. Defense in Depth: User verification before running workflows with secrets
2. Secrets Management: Proper use of GitHub Secrets, no hardcoded credentials
3. Comprehensive Security Scanning: Detailed custom security instructions covering OWASP Top 10
4. Fail-Safe Notifications: Slack alerts on security check failures
5. Environment Isolation: Proper gitignore configuration for sensitive files
6. Test Improvements: Replacing API calls with mocks improves test reliability
7. Concurrency Control: Prevents duplicate workflow runs on same PR
8. Timeout Boundaries: Prevents runaway workflows

---

Summary & Recommendations

Security Score: ✅ PASS (with minor recommendations)

Severity Breakdown:

• Critical: 0 (blocking issues)
• High: 2 (should address before merge)
• Medium: 5 (consider addressing)
• Low: 4 (informational)

Must-Fix Before Merge

1. Verify id-token: write permission is required - Remove if not needed by Claude Code Action
2. Consider fork protection - Add explicit check to prevent workflow runs on forked PRs for additional safety

Recommended Improvements

1. Add error handling to pre-commit hook to prevent commit blocking on tool failures
2. Pin @deriv-com/shiftai-cli to exact version for reproducibility
3. Reduce workflow timeout after monitoring actual execution times
4. Document security testing procedures in CLAUDE.md

Safe to Merge After

Addressing the two "must-fix" items above. The workflow security measures are generally well-designed, and the addition of comprehensive security scanning is a significant improvement to the project's security posture.

---

Specific File Reviews

.github/workflows/claude.yml

• ✅ Proper permissions scoping (mostly)
• ✅ User verification before secrets access
• ✅ Concurrency control
• ⚠️ Review id-token: write necessity
• ⚠️ Consider explicit fork protection

.github/workflows/security-nclc-review.yml

• ✅ Comprehensive security checklist
• ✅ Draft PR exclusion
• ✅ Failure notifications
• ✅ Timeout set appropriately (10 min)
• ✅ Good concurrency control

.husky/pre-commit & .husky/post-commit

• ⚠️ Pre-commit should handle CLI failures gracefully
• ✅ Post-commit always exits 0 (non-blocking)

package.json & package-lock.json

• ⚠️ Verify @deriv-com/shiftai-cli source and security
• ⚠️ Consider exact version pinning
• ✅ Husky integration properly configured

.gitignore

• ✅ Excellent additions for .env files
• ✅ AI tool configs properly excluded

CLAUDE.md

• ✅ Comprehensive and security-aware
• ✅ Good coding standards and patterns
• ✅ Clear architecture documentation

Test files

• ✅ Good refactoring to use mocked data
• ✅ Removes external dependencies
• ✅ Improves test reliability

---

[github-actions]

Preview Link: https://pr-1209.smarttrader-preview.pages.dev

Name	Result
Build status	Completed ✅
Preview URL	Visit Preview
Action URL	Visit Action