Secret trong Git, kích thước PR và checklist review Trung cấp

Trang này trả lời: lỡ commit secret thì làm gì theo thứ tự nào, phòng ngừa ra sao, và PR nên nhỏ cỡ nào để review được thật.

Không bao giờ commit secret

.env, private key, service account JSON, token trong file test — một khi đã push là coi như đã lộ.

Nếu đã lộ: rotate trước, rewrite sau

Đang tải diagram…

1. Revoke/rotate secret ngay (đổi DB password, thu hồi API key, tạo key mới). GitHub docs: sau khi rotate, secret cũ không còn dùng được, và có khi không cần rewrite history nữa.
2. Kiểm tra log truy cập của hệ thống bị lộ trong khoảng thời gian secret công khai.
3. Nếu vẫn cần xoá khỏi lịch sử (ví dụ dữ liệu cá nhân, không thể rotate): dùng git filter-repo như GitHub docs hướng dẫn.

# cần git-filter-repo ≥ 2.47 để có --sensitive-data-removal
git filter-repo --sensitive-data-removal --invert-paths --path config/prod.env
# hoặc thay chuỗi trên toàn bộ lịch sử
git filter-repo --sensitive-data-removal --replace-text ../passwords.txt

Vì sao rewrite không đủ (theo GitHub docs):

• Không xoá được trong clone và fork của người khác.
• Commit cũ vẫn truy cập được qua SHA trong cached view trên GitHub, và qua pull request tham chiếu tới nó (refs/pull/* là read-only, cần GitHub Support).
• Đồng nghiệp pull rồi push từ clone cũ, hoặc một merge commit, có thể đưa dữ liệu trở lại.
• Mọi hash từ điểm đó đổi; chữ ký commit bị mất; PR đang mở bị xáo trộn.

Kẻ tấn công quét repo public liên tục; từ lúc push tới lúc bị khai thác có thể rất ngắn. Thứ tự luôn là: rotate → rồi mới dọn lịch sử.

Phòng ngừa

Lớp	Làm gì	Lưu ý
.gitignore	Ignore .env*, *.pem, node_modules/, dist/, coverage/	File đã được track không bị ảnh hưởng — phải git rm --cached <file> trước
Ignore cá nhân	Pattern editor, OS đặt ở core.excludesFile (mặc định ~/.config/git/ignore)	Đừng làm bẩn .gitignore của project
.env.example	Commit file mẫu với key rỗng thay cho .env thật
Pre-commit hook	Secret scanner (ví dụ gitleaks), lint, format trên file staged	.git/hooks không được version hoá; dùng Husky/lefthook hoặc core.hooksPath để chia sẻ
CI	Chạy lại cùng kiểm tra, kèm secret scanning phía server nếu nền tảng hỗ trợ	Hook local bỏ qua được bằng --no-verify

Signed commits (ngắn gọn)

Ai cũng có thể đặt user.email thành email của bạn. Chữ ký chứng minh commit thật sự đến từ key của bạn. GitHub hỗ trợ GPG, SSH và S/MIME; đơn giản nhất là ký bằng SSH key sẵn có:

git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub
git config --global commit.gpgSign true

• Đăng ký key đó là “signing key” trên GitHub, rồi bật branch protection “require signed commits” nếu team cần.
• Lưu ý: “Rebase and merge” trên GitHub tạo commit mới mà GitHub không ký thay bạn được.

PR: kích thước và cách review

Góc nhìn từ người review hàng chục PR mỗi tuần:

Quy tắc	Vì sao
PR nhỏ: mục tiêu dưới ~400 dòng thay đổi thật (không tính lockfile, code sinh tự động)	Review kỹ được trong một lần ngồi; PR 2.000 dòng nhận “LGTM” vì người review bỏ cuộc
Một PR một mục đích	Revert, cherry-pick sang hotfix dễ
Tách PR refactor/rename ra trước	PR tính năng sau đó chỉ còn logic thật
Mô tả PR: vấn đề, cách giải, cách test, rủi ro, rollback	Reviewer không phải đoán; on-call đọc lại khi có sự cố
Thay đổi DB/API nhiều bước: chia PR theo expand-contract	Mỗi PR deploy được độc lập
Tự review diff của mình trước khi request	Bắt được debug log, file thừa, secret

Con số 400 dòng là kinh nghiệm, không phải chuẩn. Điều quan trọng là PR đủ nhỏ để reviewer hiểu toàn bộ.

Khi review:

• Tách rõ mức độ: blocking (bug, bảo mật, mất dữ liệu) vs nit: (style, đặt tên). Đừng block PR vì nit.
• Hỏi thay vì ra lệnh: “Nếu items rỗng thì sao?” hiệu quả hơn “Sai rồi”.
• Comment vào code, không vào người viết. Khen đúng chỗ khi thấy cách làm hay.
• Style và format để tool lo (Prettier, ESLint, CI); con người review thiết kế, tính đúng, khả năng vận hành.
• Phản hồi trong vòng một ngày làm việc. PR nằm chờ 3 ngày sẽ phình to và conflict.

Chi tiết hơn về review: cụm Code review.

Squash merge và một hotfix không cherry-pick được

Team 8 người, service thanh toán NestJS, GitHub flow với squash merge. Một PR 1.900 dòng gồm: đổi tên module, nâng Prisma, và thêm luồng hoàn tiền một phần. Sau khi merge, luồng hoàn tiền tính sai phí với giao dịch đa tiền tệ.

Muốn rollback riêng tính năng hoàn tiền: không được. Squash đã gộp cả rename và nâng Prisma vào một commit, revert là đụng migration. Team phải viết hotfix forward dưới áp lực.

Sau sự cố, họ đổi quy tắc:

1. Rename/upgrade dependency luôn là PR riêng, merge trước.
2. Tính năng mới sau feature flag, nên “rollback” chỉ là tắt flag.
3. PR quá ~400 dòng logic phải có lý do trong mô tả.

Khi nào áp dụng / ngoại lệ

Quy tắc	Ngoại lệ hợp lý
Không rebase branch đã chia sẻ	Cả team thống nhất và mọi người cùng reset lại (ví dụ xoá secret)
Squash merge	PR có các commit atomic có chủ đích (refactor → feat) mà bạn muốn giữ: dùng rebase-merge hoặc merge commit
Trunk-based	Sản phẩm phải bảo trì nhiều version song song: release branch dài hạn hoặc GitFlow
Conventional Commits	Repo cá nhân, prototype bỏ đi. Với repo team và thư viện publish, nên dùng — nhất là khi sinh changelog tự động
Revert thay vì reset trên main	Không có ngoại lệ trên branch chung

Checklist code review cho Git

1. Mỗi commit atomic, build và test pass; refactor tách khỏi thay đổi hành vi.
2. Message theo Conventional Commits; subject ≤ ~50 ký tự; body nói vì sao; breaking change có ! hoặc BREAKING CHANGE:.
3. Branch sống không quá 1-2 ngày; việc dài dùng feature flag.
4. Không có commit wip/fix review lẻ: đã --fixup + rebase -i --autosquash.
5. Force push chỉ trên branch của mình, luôn với --force-with-lease; branch protection chặn force push lên main.
6. Không có secret, .env, file build trong diff; pre-commit hook và CI cùng quét secret.
7. PR nhỏ, một mục đích, mô tả có cách test và cách rollback.
8. Sự cố trên main xử lý bằng revert, không reset + force push.

Tiếp theo: Lỗi hay gặp.