CI hardening: pin GitHub Actions to SHAs and define explicit job permissions#3173
CI hardening: pin GitHub Actions to SHAs and define explicit job permissions#3173SuyashJain17 wants to merge 11 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR makes two targeted improvements: CI/CD hardening in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/wrappers/provider.tsx (1)
22-28: Consider guarding the cleanup to run only once.The localStorage cleanup logic is correct and prevents NextThemes from applying an invalid theme class. However, it currently executes on every render of the
Providerscomponent. While the performance impact is minimal since this component typically mounts once, you could add a guard to ensure it runs only once:🔎 Optional refactor to run cleanup once
+import React, { Suspense, useRef } from 'react' export function Providers({ children }: { children: React.ReactNode }) { + const hasCleanedTheme = useRef(false) + // Prevent next-themes from applying an invalid persisted theme class - if (typeof globalThis !== 'undefined' && 'localStorage' in globalThis) { + if (!hasCleanedTheme.current && typeof globalThis !== 'undefined' && 'localStorage' in globalThis) { const stored = globalThis.localStorage.getItem('theme') if (stored?.includes(' ')) { globalThis.localStorage.removeItem('theme') } + hasCleanedTheme.current = true }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/run-ci-cd.yamlfrontend/src/wrappers/provider.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-21T19:03:59.068Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1139
File: .github/workflows/setup-backend-environment/action.yaml:16-27
Timestamp: 2025-12-21T19:03:59.068Z
Learning: Composite actions (runs: using: composite) execute as steps within the calling job's context and can access the job context, including job.services.* properties (e.g., job.services.<service_id>.id, job.services.<service_id>.ports). Service containers must be defined at the job level, but a composite action's steps can reference them via the job context.
Applied to files:
.github/workflows/run-ci-cd.yaml
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/page.tsx:59-61
Timestamp: 2025-07-13T11:29:25.245Z
Learning: In Next.js 13+ app router, components with the 'use client' directive run entirely on the client side and don't require window object existence checks or SSR hydration considerations. Direct access to window.location and other browser APIs is safe in client components.
Applied to files:
frontend/src/wrappers/provider.tsx
🧬 Code graph analysis (1)
frontend/src/wrappers/provider.tsx (1)
frontend/src/hooks/useDjangoSession.ts (1)
useDjangoSession(10-83)
🔇 Additional comments (3)
.github/workflows/run-ci-cd.yaml (3)
131-131: LGTM! Typo fixed.Good catch fixing "Denendencies" to "Dependencies" in the job name.
253-253: LGTM! Explicit permissions follow least-privilege principle.Adding an empty permissions block to the
set-release-versionjob is the correct approach. This job only executes shell commands and sets output variables, requiring no special GitHub token permissions. This change enhances security by making permissions explicit and preventing any unintended access.
46-46: SHA pinning is correct.The pinned SHA
9255dc7a253b0ccc959486e2bca901246202afebcorresponds to actions/cache v5.0.1, a stable release. This follows security best practices and is consistent with other actions pinned in the workflow.
|
Closing this PR as it was unintentionally based on a feature branch, which pulled in unrelated commits. |
1 participant
Proposed change
Resolves #3166
This PR hardens the CI/CD workflow by aligning it with GitHub Actions security best practices.
Summary of changes
permissionsto enforce least-privilege access and improve clarityNotes
Checklist
make check-testlocally and all tests passed