Repository navigation
chore(gitignore): ignore root artifacts written by Make targets - #1222
Conversation
make build-lambda, test-coverage, complexity-report and the security-scan-* targets write bootstrap, coverage.html, complexity-report.txt and gosec/trivy/tfsec-report.json at the repo root, and none of them was ignored, so a `git add -A` after a build or scan could commit the multi-MB Lambda binary or a scanner report. Add root-anchored patterns for all six so same-named files elsewhere in the tree stay trackable, and add scripts/test-gitignore-make-artifacts.sh (run by a new CI job) asserting each root artifact is ignored by .gitignore itself, the same name one level down is not, and the Makefile still writes each file. Closes LeanerCloud#279 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the .github/workflows/ci.yml conflict: main added the migration-down-gate job (LeanerCloud#1084) at the same spot as this branch's gitignore-make-artifacts job. Keep both jobs and both ci-success needs entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es it Since LeanerCloud#1145 (LeanerCloud#340), `make security-scan-go` runs scripts/run-gosec.sh, which prints text findings or writes SARIF into a given directory; nothing writes gosec-report.json at the repo root any more. Drop the /gosec-report.json rule and its test row. The test's "still in the Makefile" check kept passing for that name only because the `make clean` rm line still lists it, which is the stale guard the check was meant to catch. Skip `rm` lines when looking for the name, so a target that stops writing a file fails the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged origin/main to resolve conflicts in Conflict (
Semantic follow-up from main (
Head: Verification on the merged tree (Linux)
|
Brings in main's CI fixes (LeanerCloud#1236, LeanerCloud#1252) and CI changes since, so this PR gets a fresh CI run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent review 2 (claude-fleet, cold): APPROVE 6124b1a What the +128 is: the title is only the last commit. Against main, the PR is the whole #279 fix (09743dd). It adds:
The "drop gosec-report.json" commit (6124b1a) only removes a rule this branch added itself. Main never ignored Findings (none blocking):
"No Make target or script writes gosec-report.json" holds, including after #1145's rewrite. I ran git grep on head and origin/main across the Makefile,
The writers that remain produce other files:
Mutation probes, in a scratch
Verified locally:
Reviewed by an independent agent with no author context (second reviewer). Review only: nothing was pushed, and the GitHub review state is unchanged. |
|
Independent review 2 (claude-fleet, cold), re-review: APPROVE c9eb4b3 This is a delta from my APPROVE at 6124b1a. The only new commit is a merge of origin/main (503eb2e):
Verified locally on a
My earlier findings and notes still apply unchanged. Same reviewer as the earlier review, re-reviewing the new head. Review only: nothing was pushed, and the GitHub review state is unchanged. |
|
VERDICT: REQUEST_CHANGES c9eb4b3 The content is correct; the blockers are a merge conflict with current main and one overstated guard. Findings:
Issue #279 check: the five root artifacts the Makefile writes today (bootstrap at Makefile:58, coverage.html at :81, complexity-report.txt at :152, trivy-report.json at :170, tfsec-report.json at :179) are now ignored by root-anchored rules. gosec-report.json is left out, which is justified: nothing in the tree writes it any more (run-gosec.sh writes SARIF only into --sarif-dir, and security-scan.sh writes into security-reports/). Only the stale Attacked:
|
…os (#349, slice 1) (#1307) * chore(ci): make local gates fail honestly and pin pre-commit hook repos First slice of the 2026-07-28 review findings for this repo: - Makefile: lint, security-scan-docker and security-scan-snyk fail when the tool is missing and stop on the tool's own exit status instead of a trailing echo; drop the retired tfsec target; terraform-validate validates the flat terraform/environments/<cloud> roots instead of a nonexistent */dev glob. scripts/test-make-gates.sh covers all of this with stub tools and runs in the lint job. - Makefile.terraform: clean-locks needs CONFIRM_CLEAN_LOCKS=yes. - database-migration.yml: stop writing migrate's stderr into GITHUB_ENV. - .pre-commit-config.yaml: pin the three third-party hook repos to commit SHAs and let check-yaml see workflow files. - .golangci.yml: gocyclo min-complexity 10, matching the CI gate. - ci.yml: tfvars fmt checks no longer swallow a non-zero exit. - .env.example: no working admin password or API key. - .dockerignore: exclude the test compose/Dockerfile pair. - .trivyignore/.snyk: correct the AVD-AWS-0013 and GCP-0015 reasons and drop .snyk keys the Snyk CLI never reads. Refs #349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(make): keep the deprecated tfsec target, out of security-scan Deleting it breaks #1222's gitignore test, which expects the Makefile to still write tfsec-report.json. The target now fails honestly like the others; removing it can follow once #1222 merges. Refs #349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Brings the branch up to date with main so the PR is mergeable again. ci.yml conflicted where LeanerCloud#1119 added the gcp-import-dev-state job and LeanerCloud#1112 added rollback-ecr-registry to ci-success needs. Keep main's jobs and needs entries as they are and add this PR's gitignore-make-artifacts job and needs entry after them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The "Makefile still writes <name>" check matched any non-rm line, including the `@echo "...: <name>"` status lines. Renaming the coverage.html, trivy-report.json, tfsec-report.json or complexity-report.txt output therefore still passed, because the echo kept the old name, contrary to what the header comment and the PR say. Require the name to follow -o, --out, --output, tee or a > / >> redirection instead. Renaming any of the five outputs in the Makefile now fails the check, and the rm-line filter is no longer needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Reply to the REQUEST_CHANGES review on c9eb4b3, point by point:
The PR body's claim, that a renamed or dropped output fails the test, is now true as written. Checks on 5b874c8: New head: 5b874c8 |
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE Blocking:
Non-blocking:
Attacked:
Body-stale:
Reviewer: rB25. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths. |
|
Local verification at |
|
Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on |
Brings the branch up to date with main (f316add). Conflicted files: - .github/workflows/ci.yml: main added the gcp-import-dev-state (LeanerCloud#1119) and gitignore-make-artifacts (LeanerCloud#1222) shell-only jobs in the same spot where this branch adds tf-deploy-wrapper. Kept all three jobs, with tf-deploy-wrapper after main's two, and listed all three in ci-success's needs alongside main's new vpc-peering-delete-scope, tfstate-concurrency-group and rollback-ecr-registry entries. Makefile.terraform merged cleanly: main's clean-locks confirmation (LeanerCloud#1307) and this branch's frontend-only change touch different targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Root cause
Five documented Make targets write files at the repo root, and
.gitignorecovered none of them. After a build, coverage run or scan,git add -Awould pick them up. The worst case is the multi-MB linux/arm64bootstrapLambda binary.coverage.outwas already covered by*.out, andbin/cudly-serverby/bin/.bootstrapmake build-lambdacoverage.htmlmake test-coveragetrivy-report.jsonmake security-scan-dockertfsec-report.jsonmake security-scan-terraformcomplexity-report.txtmake complexity-reportFix
.gitignore: one root-anchored pattern per file (/bootstrap,/coverage.html,/trivy-report.json,/tfsec-report.json,/complexity-report.txt). The issue also listsgosec-report.json, but since ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145make security-scan-gorunsscripts/run-gosec.sh, which writes no root file, so it is left out. The issue suggests a*-report.jsonglob, but I used exact anchored names instead. Unanchored, the glob would also ignore any nested*-report.jsonfixture someone adds later, and the probe below shows it doing so.scripts/test-gitignore-make-artifacts.sh(new): checks each file in both directions withgit check-ignore -v --no-index..gitignoreitself, so a developer's global excludes file or.git/info/excludecan't make it pass locally.some/nested/dir/<name>is not ignored by.gitignore, so the patterns stay anchored.Makefileon a line other than anrmline, so if a target is renamed or stops writing the file the test fails instead of guarding a file that is no longer produced. (make cleanstill listsgosec-report.json, which is whyrmlines are skipped.).github/workflows/ci.yml: new shell-only jobgitignore-make-artifactsruns the script, and it is added toci-successneeds. It follows the same structure as the other self-test jobs: pinned checkout SHA,persist-credentials: false,contents: read.Closes #279
Verification
All evidence comes from local runs against the real
.gitignore: first on macOS, then re-run on Linux on the tree merged with origin/main (6124b1a3). I did not run the heavy Make targets (build, coverage, scanners) to produce the files; I used emptytouchstand-ins with the same paths.Fail before (the test script run against
.gitignorewithout this PR's rules, on the merged tree):Pass after:
Stand-in files present: after
touch bootstrap coverage.html trivy-report.json tfsec-report.json complexity-report.txtat the root,git status --porcelainshowed only this PR's own edits, andgit status --porcelain --ignoredlisted all five as!!. I deleted the stand-ins afterwards.No tracked file becomes ignored:
git ls-files -ci --exclude-standardgives the same output before and after the change (diffidentical, 13 entries). Those 13 are all pre-existing; see Out of scope. No tracked file anywhere in the tree has any of the five names (git ls-files | grep).Mutation probes (each applied to
.gitignore, then reverted):/bootstrapchanged to unanchoredbootstrapbootstrapignored/trivy-report.jsonchanged to the*-report.jsonglob/tfsec-report.jsonline deletedtfsec-report.jsonnot ignored/bootstrapdeleted, but present in a globalcore.excludesFile.gitignoregosec-report.jsonrow restored in the test (onlymake clean'srmline names it)tfsecwriter lines deleted from theMakefile(rmline kept)Other checks:
shellcheck scripts/test-gitignore-make-artifacts.sh: cleanactionlint .github/workflows/ci.yml: cleanuvx zizmor==1.29.0 --offline --persona=pedantic --min-severity=medium .github/workflows/ci.yml: no findings (7 ignored, 1 suppressed; origin/main gives the same counts). Run before the main merge; not re-run sincegit diff --check: clean; files end with a newline--register-awsin a freshgit initrepo with an isolated HOME, scanning the three changed files with the repo's.gitallowed): clean; a planted fake secret key was caught as a positive controlbash scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL on the merged tree./internal/mocksdon't applyOut of scope (noted, not changed)
make cleanremoves the other four files but notcomplexity-report.txt, and it still removesgosec-report.json, which no target writes since ci(gosec): run one rule set from a shared script in the hook, Makefile and CI #1145. Both are one-line Makefile changes; I left them out to keep this PR to the ignore rules.git ls-files -ci --exclude-standardalready lists 13 tracked files ignored by older patterns. These are the unanchoredcleanup-lambdaandclear-rate-limitpatterns matching source directories, and.terraform.lock.hclmatching committed lock files.cloudformation/stacks/CUDly/template.yamlappears only on case-insensitive checkouts such as macOS, where it matchescudly. Tracked files stay tracked, but new files under those paths would be silently ignored. This PR leaves that set unchanged.Labels
Could a maintainer please mirror the issue's triage labels onto this PR:
impact/internal,type/chore,triaged,priority/p3,severity/low,urgency/eventually,effort/xs.🤖 Generated with Claude Code