Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 62 additions & 4 deletions .github/workflows/secret-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,27 @@ jobs:
./gitleaks version

- name: Run gitleaks (HARD FAIL on any leak)
# `detect` scans the git history; `--exit-code 1` makes a finding
# fail the job. Findings are redacted in the log and written to the
# SARIF artifact for the Security tab. Suppress provable
# non-secrets only in .gitleaks.toml, never by relaxing this gate.
# `detect` scans the COMMIT HISTORY reachable from HEAD, not the
# checked-out files. Two consequences worth stating, because both have
# cost real time:
#
# 1. Deleting the value and committing the deletion does NOT clear
# this. The commit that introduced it is still on the branch, and
# commits are what is scanned. The failure survives the fix, which
# reads like the fix not being picked up. The `if: failure()` step
# below says so at the moment it happens.
# 2. Another contributor's branch cannot fail your pull request.
# `fetch-depth: 0` means "all of this ref's history", not "all
# branches": actions/checkout fetches only the pull-request ref,
# so no other branch is present for gitleaks to read. A
# cross-branch failure was reported once; the run logs show each
# pull request scanning main's commit count plus its own (289 + n
# at the time), never the whole repository.
#
# `--exit-code 1` makes a finding fail the job. Findings are redacted
# in the log and written to the SARIF artifact for the Security tab.
# Suppress provable non-secrets only in .gitleaks.toml, never by
# relaxing this gate.
run: |
./gitleaks detect \
--source . \
Expand All @@ -66,6 +83,47 @@ jobs:
--report-path gitleaks.sarif \
--exit-code 1

- name: Explain what a finding means
if: failure()
run: |
cat <<'EOF'
============================================================
gitleaks scanned this branch's COMMIT HISTORY, not its files.
============================================================

If you already removed the value, that is why this still fails: the
commit that added it is still on the branch, and the scan reads
commits. Taking it out of the working tree does not take it out of
history.

To clear it the commit has to go. From a clean tree, with no one
else building on your branch:

git fetch origin
git rebase origin/main # FIRST, before collapsing anything
git reset --soft origin/main # your whole diff becomes one staged change
git commit # one commit, without the bad value
git push --force-with-lease

Rebase before the reset, not after. `git reset --soft` moves the
branch to wherever origin/main points NOW while keeping your tree, so
if main advanced after you branched, the single commit you create
silently reverts everything merged in between. That has happened
here: 125 lines from two merged pull requests were undone this way,
and the only signal was a diffstat with far more deletions than the
change could account for. Before committing, check
`git diff --stat origin/main`, and check that
`git diff --name-only origin/main` lists no file you did not touch.

If the value is a real credential: rotate it. It reached a remote, so
treat it as disclosed no matter what the history says afterwards.

If it is provably not a secret, add it to .gitleaks.toml with the
reason. Never allowlist a value you cannot prove is safe.

More: docs-site/docs/contributor-guide/coding-standards.md
EOF

- name: Upload gitleaks SARIF
if: always()
uses: actions/upload-artifact@v4
Expand Down
24 changes: 24 additions & 0 deletions docs-site/docs/contributor-guide/coding-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,30 @@ The format is:

Reviewers reject suppressions that lack a justification or whose justification does not address the rule. If multiple lines need the same suppression, lift the offending logic into a single function and suppress once.

## `secret-scan` reads history, not your files {#secret-scan-reads-history}

`gitleaks detect` scans the commits reachable from your branch's HEAD. It does not scan the checked-out tree. That distinction is invisible until the check fails, and then it explains a failure that otherwise looks impossible.

**Deleting the value does not clear the finding.** The commit that introduced it is still on the branch, so the scan still sees it. If you remove the value, commit, push, and the check fails again with the same finding, nothing went wrong with your fix: it was never the problem being reported. Somebody has already lost two rounds of CI to this, editing the working tree a second and third time.

**Another contributor's branch cannot fail your pull request.** `fetch-depth: 0` reads as "fetch everything", but it means all of *this ref's* history: `actions/checkout` fetches only the pull-request ref, so no other branch is in the checkout to be read. The run logs bear it out, with each pull request scanning main's commit count plus its own rather than the whole repository.

To clear a finding, the commit carrying it has to go. From a clean tree, with nobody else building on your branch:

```bash
git fetch origin
git rebase origin/main # FIRST, before collapsing anything
git reset --soft origin/main # your whole diff becomes one staged change
git commit # one commit, without the value
git push --force-with-lease
```

Rebase before the reset. `git reset --soft origin/main` moves your branch to wherever `origin/main` points **now** while keeping your working tree, so if main advanced after you branched, the single commit you create reverts everything merged in between. It happens silently, because your own files are all still correct. This has happened in this repository: 125 lines from two merged pull requests were undone that way, and the only clue was a diffstat showing far more deletions than the change could account for. Before committing, read `git diff --stat origin/main`, and confirm `git diff --name-only origin/main` lists nothing you did not touch.

If the value was a real credential, rotate it. It reached a remote, so treat it as disclosed regardless of what the history looks like afterwards. Rewriting history hides it from the next scan; it does not un-publish it.

Only add a value to `.gitleaks.toml` when you can prove it is not a secret, and record the proof in the comment beside it. "It is only used in CI" is context, not proof.

## AI review comments — advisory, never a gate

A pull request may pick up a comment headed **AI security review (findings-driven)**. It comes from `.github/workflows/ai-review.yml`, which re-runs `semgrep` over the files you changed — without the severity filter and the `--error` flag the SAST gate uses — and asks a model to sort what it flagged into true and false positives.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,30 @@ expression = re.compile(user_input) # nosemgrep: regex-from-user-input: `is_saf

리뷰어는 정당화가 없거나 룰을 다루지 않는 suppress를 거절합니다. 같은 suppress가 여러 라인에 필요하면 문제 로직을 단일 함수로 추출해 한 곳에서 suppress하세요.

## `secret-scan`은 파일이 아니라 이력을 읽습니다 {#secret-scan-reads-history}

`gitleaks detect`는 브랜치 HEAD에서 도달할 수 있는 커밋을 검사합니다. 체크아웃된 작업 트리를 보는 것이 아닙니다. 이 차이는 검사가 실패하기 전까지 드러나지 않는데, 실패하고 나면 달리 설명되지 않는 현상을 설명해 줍니다.

값을 지워도 발견은 사라지지 않습니다. 그 값을 넣은 커밋이 브랜치에 남아 있으므로 검사는 여전히 그것을 봅니다. 값을 지우고 커밋해서 푸시했는데 같은 발견으로 또 실패한다면 수정이 잘못된 것이 아닙니다. 애초에 보고된 대상이 작업 트리가 아니었을 뿐입니다. 여기서 CI를 두 번 더 돌리며 작업 트리만 고친 사례가 이미 있습니다.

다른 기여자의 브랜치가 내 풀 리퀘스트를 실패시키지는 않습니다. `fetch-depth: 0`은 "전부 가져온다"처럼 읽히지만 해당 ref의 이력 전체라는 뜻입니다. `actions/checkout`은 풀 리퀘스트 ref만 가져오므로 다른 브랜치는 체크아웃에 아예 없습니다. 실행 로그도 같은 것을 보여 줍니다. 각 풀 리퀘스트는 저장소 전체가 아니라 main의 커밋 수에 자기 커밋을 더한 만큼만 검사합니다.

발견을 없애려면 그 값을 담은 커밋 자체를 없애야 합니다. 작업 트리가 깨끗하고 브랜치 위에서 작업하는 사람이 없을 때 이렇게 합니다.

```bash
git fetch origin
git rebase origin/main # 합치기 전에 리베이스를 먼저
git reset --soft origin/main # 전체 diff가 스테이지된 변경 하나가 됩니다
git commit # 문제 값이 없는 커밋 하나
git push --force-with-lease
```

리베이스가 reset보다 먼저입니다. `git reset --soft origin/main`은 작업 트리를 유지한 채 브랜치를 지금 시점의 `origin/main`으로 옮깁니다. 그래서 브랜치를 만든 뒤 main이 앞서 나갔다면, 새로 만든 커밋 하나가 그사이 머지된 변경을 전부 되돌립니다. 내 파일은 모두 멀쩡하므로 이것은 조용히 일어납니다. 이 저장소에서 실제로 일어난 일입니다. 머지된 풀 리퀘스트 두 개의 125줄이 그렇게 사라졌고, 단서는 변경 내용에 비해 삭제 줄 수가 지나치게 많은 diffstat 하나뿐이었습니다. 커밋하기 전에 `git diff --stat origin/main`을 읽고, `git diff --name-only origin/main`에 자신이 건드리지 않은 파일이 없는지 확인하세요.

값이 실제 자격증명이었다면 교체하세요. 원격에 올라간 이상 이후 이력이 어떻게 보이든 노출된 것으로 다뤄야 합니다. 이력을 다시 쓰는 것은 다음 검사에서 감추는 것이지 공개를 되돌리는 것이 아닙니다.

`.gitleaks.toml`에 값을 추가하는 것은 그것이 시크릿이 아님을 증명할 수 있을 때만이고, 근거를 옆 주석에 남깁니다. "CI에서만 쓴다"는 것은 정황이지 증명이 아닙니다.

## AI 리뷰 코멘트 — 참고용이며 게이트가 아닙니다

PR에 **AI security review (findings-driven)** 제목의 코멘트가 붙을 수 있습니다. `.github/workflows/ai-review.yml`이 남기는 것으로, 바뀐 파일만 대상으로 `semgrep`을 다시 돌린 뒤 — SAST 게이트가 쓰는 severity 필터와 `--error` 플래그는 빼고 — 탐지된 항목을 모델이 실제 취약점과 오탐으로 나눈 결과입니다.
Expand Down
Loading