Code Review Guidelines Gitlab

Standard Operating Procedure: Code Review Guidelines GitLab

Document Identifier: SOP-ENG-GL-042
Classification: Internal Enterprise Standard

---

1. Document Control & Revision History

---

2. Executive Summary & Scope

2.1 Clinical Statement of Operational Boundaries

This Standard Operating Procedure (SOP) defines the mandatory, end-to-end lifecycle for peer review, quality verification, and secure merging of source code within the enterprise GitLab ecosystem. Unreviewed or improperly verified code introduces unacceptable risk to production stability, intellectual property, and regulatory posture.

2.2 Regulatory Standards & Compliance

Adherence to this SOP ensures compliance with:

• SOC 2 Type II: Trust Services Criteria (Security, Availability, Confidentiality).
• ISO/IEC 27001: Information Security Management (Access Control, Secure Development Lifecycle).
• PCI-DSS v4.0: Requirement 6 (Secure Software Development and Code Review).

All engineering personnel, contractors, and automated pipelines executing version control operations within GitLab must execute every step outlined herein. Non-compliance results in immediate revocation of merge privileges and an internal security audit.

---

3. Prerequisites & Tool Requirements

To execute or participate in the code review workflow, personnel and systems must satisfy the following baseline prerequisites:

• Software & Tooling Access:
  • GitLab Enterprise Edition (EE) v16.0+ with Ultimate License tier.
  • Git CLI v2.38+ configured with SSH keys (ed25519) and commit signing via GPG/SSH.
  • IDE integration (VS Code, JetBrains Suite) with GitLab Workflow extension installed and authenticated via Personal Access Tokens (PAT) with api and read_api scopes.
• Network & Security Requirements:
  • Mandatory multi-factor authentication (MFA/WebAuthn) enforced on all GitLab accounts.
  • Access restricted to corporate Virtual Private Network (VPN) or Zero-Trust Network Access (ZTNA) gateway.
• Certifications & Training:
  • Completion of Enterprise Secure Coding Standards (SEC-101).
  • Completion of GitLab Advanced Merge Request Management (ENG-202).
• Pipeline Infrastructure:
  • Operational GitLab CI/CD runners with Docker-in-Docker (DinD) execution capability, static analysis linters, Secret Detection, Software Composition Analysis (SCA), and Dynamic Application Security Testing (DAST) engines.

---

4. Roles & Responsibilities (RACI Matrix Table)

Definitions: Responsible (The doer), Accountable (The owner), Consulted (In-the-loop advisor), Informed (Kept updated).

---

5. Step-by-Step Operational Procedure

Phase 1: Author Preparation and Submission

• 1.1 Verify that the local workspace is synchronized with the upstream main/master branch using git pull --rebase origin main.
• 1.2 Create a descriptive, ticket-linked feature branch adhering to naming conventions (feature/JIRA-1234-short-description, bugfix/JIRA-5678-fix-null-pointer).
• 1.3 Implement atomic, well-documented commits with descriptive commit messages referencing the tracking ID (e.g., feat(auth): implement OAuth2 token refresh mechanism [JIRA-1234]).
• 1.4 Execute local unit tests, integration tests, and static linting checks (e.g., SonarQube Scanner, ESLint, Flake8) to ensure zero local failures.
• 1.5 Push the branch to the remote GitLab repository: origin push -u origin feature/JIRA-1234-short-description.
• 1.6 Navigate to GitLab, open the Merge Request (MR) interface, and select the correct project template (.gitlab/merge_request_templates/Default.md).
• 1.7 Populate the MR description fully: include Business Value, Architecture Impact, Testing Evidence, and explicit links to Jira/ServiceNow tickets.
• 1.8 Assign the appropriate Reviewer(s) based on CODEOWNERS definitions and set the Milestone and Labels (security-review, backend, breaking-change).

Phase 2: Automated Pipeline Validation

• 2.1 Verify that the GitLab CI/CD pipeline triggers automatically upon MR creation.
• 2.2 Confirm successful completion of the .lint stage (YAML, Markdown, Code style linters).
• 2.3 Confirm successful completion of the .test stage (Unit, Integration, and Regression test suites with 80%+ code coverage maintenance).
• 2.4 Verify that Security Scanners (SAST, Secret Detection, Dependency Scanning, Container Scanning) report Zero Critical or High vulnerabilities.
• 2.5 Resolve any pipeline failures immediately by pushing corrective commits; do not request human review while the CI/CD pipeline is in a failed (red) state.

Phase 3: Peer Review & Technical Audit

• 3.1 Reviewer accesses the assigned MR via GitLab, reviewing the "Changes" tab using side-by-side or unified diff views.
• 3.2 Audit code architecture for adherence to SOLID principles, design patterns, and enterprise anti-pattern avoidance.
• 3.3 Inspect for security vulnerabilities: SQL injection, Cross-Site Scripting (XSS), insecure deserialization, hardcoded secrets, and proper Identity and Access Management (IAM) controls.
• 3.4 Verify performance implications: inspect for N+1 query problems, inefficient memory allocations, unindexed database queries, and unbounded loops.
• 3.5 Ensure comprehensive test coverage exists for newly introduced logic, including edge cases and negative test scenarios.
• 3.6 Utilize inline GitLab discussion threads to document specific feedback; categorize comments as [Blocking] or [Non-Blocking / Nitpick].
• 3.7 Author resolves all [Blocking] comments and replies with implementation justifications or test verification logs.
• 3.8 Reviewer re-evaluates changed lines and approves the MR utilizing the GitLab "Approve" action (formal electronic signature).

Phase 4: Final Verification and Merge Execution

• 4.1 Verify that all required approval thresholds are met (Minimum 2 peer approvals, plus 1 Security Team approval if security-review label is present).
• 4.2 Ensure pipeline is green and the branch is up-to-date with the target base branch (Resolve any merge conflicts via web interface or local rebase).
• 4.3 Select the appropriate merge method mandated by project configuration (Squash and Merge preferred to maintain a clean linear history).
• 4.4 Execute the merge action in GitLab.
• 4.5 Verify deletion of the remote feature branch post-merge.
• 4.6 Verify downstream deployment pipelines trigger successfully in staging environments.

---

6. Quality Assurance & Verification Criteria

6.1 Audit Checklist

• Code Correctness: Implements stated requirements without unintended side effects.
• Readability & Maintainability: Follows language-specific style guides; naming conventions are intuitive; complex logic contains explanatory comments.
• Test Adequacy: New unit tests added; integration tests pass; test coverage delta is $\ge 0$ (does not drop project average).
• Security Baseline: No credentials, API tokens, or PII exposed; inputs are sanitized; authorization checks validate user permissions.
• Documentation: README, API documentation (OpenAPI/Swagger), or runbooks updated if architectural contracts change.

6.2 Tolerance Thresholds & Acceptance Benchmarks

• Review Turnaround Time: Initial review response must occur within 24 business hours of MR assignment.
• Defect Density Limit: Maximum of 1 defect per 100 lines of code identified during peer review; higher defect densities require mandatory architecture re-evaluation.
• Pipeline Duration: Total CI/CD verification pipeline must execute in under 15 minutes.
• Security Zero Tolerance: 0 Critical, 0 High, $\le 2$ Medium unmitigated vulnerabilities allowed for merge approval. Medium vulnerabilities require a documented risk acceptance ticket signed by the Information Security Officer (ISO).

---

7. Practical Troubleshooting Scenarios

Scenario A: Persistent Pipeline Security Failures (Secret Detection Triggered)

• Symptom: The GitLab pipeline halts during the Secret Detection stage, flagging a committed private SSH key or API token in the diff, preventing merge execution.
• Immediate Corrective Action Protocol:
  1. Do not attempt to delete the file in a subsequent commit; historical revisions still retain the secret in the Git object database.
  2. Immediately revoke the compromised credential at the identity provider (AWS IAM, GitHub, Vault, etc.).
  3. Execute an interactive rebase locally (git rebase -i HEAD~N) or utilize git filter-repo / BFG Repo-Cleaner to permanently purge the file from the local commit history.
  4. Force-push the sanitized history to the remote branch (git push origin feature/branch --force-with-lease).
  5. Notify the Security Operations Center (SecOps) via the emergency Slack channel (#sec-incident) if the repository is public or if the token was active in production.

Scenario B: Deadlock Due to Unresponsive Code Owner / Reviewer Absence

• Symptom: An urgent hotfix MR is blocked because the assigned CODEOWNER is out of office (OOO) and has not responded within the 4-hour emergency SLA.
• Immediate Corrective Action Protocol:
  1. Verify the reviewer's status via GitLab profile or Slack out-of-office indicator.
  2. Ping the secondary backup reviewer designated in the team's rotation schedule.
  3. If no secondary reviewer is available, escalate to the Engineering Manager on duty.
  4. The Engineering Manager is authorized to manually override the CODEOWNER restriction using administrative GitLab permissions, provided a secondary peer review is completed and documented in the MR thread.
  5. File an internal operations ticket to update the CODEOWNERS file backup mapping to prevent recurrence.

Scenario C: Complex Merge Conflicts Post-Approval

• Symptom: Concurrent merges to the target main branch introduce complex merge conflicts, invalidating the previous peer review approval state due to GitLab's automatic approval reset rule.
• Immediate Corrective Action Protocol:
  1. Fetch the latest main branch locally: git fetch origin main.
  2. Rebase the feature branch onto main: git rebase origin/main.
  3. Resolve conflicts explicitly in the IDE, ensuring business logic from both changes is preserved correctly.
  4. Run local test suites to verify that conflict resolution did not break integration contracts.
  5. Push the resolved branch: git push origin feature/branch --force-with-lease.
  6. Notify the primary reviewer via comment mention that conflicts have been resolved; the reviewer must perform a quick delta-review and re-apply approval without requiring a full re-audit, provided code logic remains unchanged.