Skip to content

Commit a57c5eb

Browse files
Fix TruffleHog PR scans that silently scanned nothing (#2648)
### What does this PR do? Type of change: Bug fix The nightly `code-quality` run has failed every night since #2514 merged (e.g. [run 36648252702](https://github.com/NVIDIA/Model-Optimizer/actions/runs/36648252702/job/109676162445)), even though every PR passes. **Root cause:** on `pull_request`, `actions/checkout` does a depth-1 clone, so the PR's base commit is missing. TruffleHog logs `unable to resolve commit: object not found`, scans **0 chunks**, and exits 0. Every recent PR run shows `"chunks": 0`, so the PR secret scan has been a no-op. Nightly has no base to diff against and scans the whole tree, so it is the only run that actually checks anything. **Changes:** - `code_quality.yml`: fetch full history on PRs (`fetch-depth: 0`) so `--since-commit <base>` resolves. Nightly stays shallow, so it keeps scanning the current tree instead of all of git history. (`'0'` is a string because the number `0` is falsy in expressions and would fall through to `1`.) - `test_mlflow.py`: `# trufflehog:ignore` only applies to its own line. The marker sat on the closing `)`, not on the two credential-URI lines that were flagged (1260, 1264). I moved it onto those lines and shortened one test password so the line fits the 100-char limit; it still tests a percent-encoded `/`. Heads-up: now that PR scans actually run, PRs that add secret-looking strings will fail the scan, as intended. ### Testing - Reproduced the PR-mode scan for #2514 with full history, using trufflehog 3.97.9 (the version CI pulls): it reports the same two `URI` findings at lines 1260 and 1264 and exits 183, so it would have blocked that PR. - Filesystem scan of the whole repo on this branch with CI's flags: no findings. - `pytest tests/unit/torch/utils/test_mlflow.py -k credentials_move`: 6 passed. pre-commit passes on the changed files. ### Before your PR is "*Ready for review*" - Is this change backward compatible?: ✅ - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: N/A - Did you write any new necessary tests?: N/A - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A - Did you get Claude approval on this PR?: N/A ### Additional Information The action pulls `trufflehog:latest`, so new upstream detectors can change results. It could be pinned via the action's `version:` input; this PR leaves that unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated credential-handling test coverage to verify that percent-encoded passwords are interpreted correctly. * **Chores** * Pull-request quality checks now fetch full repository history, while checks for other events continue to use a shallow checkout. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 472c94d commit a57c5eb

2 files changed

Lines changed: 6 additions & 3 deletions

File tree

‎.github/workflows/code_quality.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ jobs:
2020
timeout-minutes: 30
2121
steps:
2222
- uses: actions/checkout@v6
23+
with:
24+
# PRs: TruffleHog needs the base commit, else it errors, scans nothing, and passes.
25+
fetch-depth: ${{ github.event_name == 'pull_request' && '0' || '1' }}
2326
- uses: ./.github/actions/ubuntu-setup
2427
- name: Secret Scanning
2528
uses: trufflesecurity/trufflehog@v3.90.5

‎tests/unit/torch/utils/test_mlflow.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,17 +1257,17 @@ def test_a_clean_close_elsewhere_does_not_mask_this_block_s_failure(fake_mlflow,
12571257
(URI, URI, None, None),
12581258
# Percent-encoded userinfo: requests decodes what it finds in a URI, so this must too,
12591259
# or the two paths authenticate as different users.
1260-
("https://al%3Ace:tok%2Fen@mlflow.example.com", URI, "al:ce", "tok/en"),
1260+
("https://al%3Ace:t%2Fk@mlflow.example.com", URI, "al:ce", "t/k"), # trufflehog:ignore
12611261
# MLflow sends basic auth only with both variables set, so half a credential has
12621262
# nowhere to go; the caller is told rather than handed a URI it would record from.
12631263
("https://tok@mlflow.example.com", None, None, None),
1264-
("https://:tok@mlflow.example.com", None, None, None),
1264+
("https://:tok@mlflow.example.com", None, None, None), # trufflehog:ignore
12651265
# urlparse reads "user:" as the scheme and leaves the credential in .path, where the
12661266
# userinfo check never sees it -- so this fails closed rather than returning it.
12671267
("user:tok@mlflow.example.com", None, None, None),
12681268
],
12691269
ids=["pair", "none", "percent-encoded", "no-password", "no-username", "no-scheme"],
1270-
) # trufflehog:ignore
1270+
)
12711271
def test_credentials_move_out_of_the_uri_into_mlflows_own_variables(
12721272
uri, stripped, username, password
12731273
):

0 commit comments

Comments
 (0)