Conversation
App Security's deterministic checks scanned files that git ignores, such as local .env files and generated output, and reported findings for files that never reach the repository. Discovery now walks the app root once and skips a path when either of two exclusion phases matches it: - Default .gitignore-style patterns for dependencies, build output, caches, test and fixture trees, and CLI-generated folders. - The untracked, ignored paths git reports, so nested .gitignore files, negations, .git/info/exclude and global excludes all apply. Tracked files that match .gitignore are still scanned. No git exclusions apply when the app isn't in a git repository, when git fails, or when an enclosing repository ignores the app folder or one of its ancestors. Git's listing skips the default directories, so git doesn't traverse trees the walker never enters. The walker prunes excluded folders, stops at nested apps, and walks dot-folders and dotfiles, so .github/ and .vscode/ are now scanned for secrets. Loading the app configuration isn't subject to exclusions. Dependabot and Renovate configuration is, so an ignored configuration file no longer counts as dependency automation. The committed-secret check no longer skips untracked, ignored files on its own, because discovery decides what is scanned. When git ignores a file that was still scanned, the finding says why: an enclosing repository ignores the app, the file is inside a nested repository, or git couldn't list ignored files.
Contributor
|
Sorry to ask, but why is a change to exclude gitignored files, a +2000 line change? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
shopify app security checkran its deterministic checks on files that git ignores, such as local.envfiles and generated output. That produced findings, including committed-secret findings, for files that never reach therepository.
WHAT is this pull request doing?
Discovery now walks the app root once and skips a path when a default pattern or git excludes it:
git ls-files --others --ignored --exclude-standard, so nested.gitignorefiles, negations,.git/info/excludeand global excludes all apply. Tracked files that match.gitignoreare still scanned..github/and.vscode/are scanned for secrets. Generated dot-folders (.next/,.shopify/,.yarn/, …) are excluded by default.Exclusions are silent, like the existing hardcoded ones: they aren't recorded in the trace or submission.
How to manually test your changes?
In a git-tracked app:
.envto.gitignore, and put a Shopify token-shaped value (shpat_followed by 32 hex characters) in.env.pnpm shopify app security check --path /path/to/app: no committed-secret finding for.env.git -C /path/to/app add -f .env, then rerun: the finding appears, because tracked files are scanned..github/workflows/deploy.ymland rerun: it's reported.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add