Warn when secret files are excluded from the push

[somaz94]

Following up on the review David and I did over email.

push already hard-excludes secret files (.env*, .pem, .key, .p12, .pfx, .netrc) so they can't be published, which is great, but it does it silently. This adds a one-line notice listing the secret files that were withheld, so you can tell at a glance that a sensitive file was kept out of the deploy (and why it's missing, if you expected it to ship).

Only secrets that would otherwise have shipped are listed. Files already excluded via .fyraignore aren't repeated, and .git / node_modules aren't descended into.

Example:

Skipped 1 secret file (excluded by default, not uploaded):
  .env

Implementation: extracted the existing secret-name check into an isSecretFile predicate (shouldSkip calls it, no behavior change) and added a pre-push scan that reports the withheld files on both the interactive and non-interactive paths.

Validation (local):

• gofmt clean, go build, go vet pass
• CGO_ENABLED=1 go test -race ./... passes, including new tests for isSecretFile, skippedSecretFiles, and warnSkippedSecrets

[somaz94]

Hey David, this is the follow-up to the review we did over email.

I pulled v0.3.8 and went through it from source. The TLS change is exactly right: the default dial verifies against the system roots now, and the insecure path is gated behind INSECURE_VERIFY with a stderr warning, so it can't happen by accident in prod. I also confirmed the broadened secret exclusion (.pem/.key/.p12/.pfx/.netrc on top of .env) and the --help port matching :50052. Build, vet, and the race test suite are green on my end.

This PR is the small "maybe worth a warning" bit from that thread. The secret exclusion works, but it's silent, so you don't find out a .env was withheld unless you go looking. It adds a one-line notice listing the secret files a push kept out, only the ones that weren't already gitignored.

No rush, and feel free to close it if you'd rather handle the UX differently. I still owe you a live deploy for round two once I get to it.

[davidchua]

Sorry for the late review! Thanks @somaz94 for the PR!

Thanks for this! I think the intention is correct, we should surface up to the user when files are implicitly removed that are not part of .fyraignore.

Happy to merge it in but just two general comments for changes.

1. I think we should send the warning into stderr instead of stdout.
2. Not blocking, but I think we should extract the dir pruning list now that there are more than one methods that is using the same logic.

Super appreciate the PR 🙇

[davidchua]

I think we should use os.Stderr instead of os.Stdout here to keep to the convention that warnings belong to Stderr.

This would also allow us to keep stdout clean for users who might want to pipe their fyra push output into their CI/CD.

[somaz94]

Done. Moved the notice to os.Stderr so stdout stays clean for piping into CI/CD.

[davidchua]

Just a small nitpick, this same rule lives in shouldSkip's isDir branch (push.go:202). Today they agree.

The hazard is what happens when they don't: if someone later adds .next/ or vendor/ to shouldSkip and forgets this function, the warning will start naming secrets inside directories that would never have shipped.

We might want to abstract this out, like a pruneDir() so there's only one definition of directories to skip.

[somaz94]

Extracted the skip list into a pruneDir() helper and pointed both shouldSkip and skippedSecretFiles at it, so they can't drift. Added a small TestPruneDir to lock the shared list.

[somaz94]

Rebased on latest main to resolve the conflicts — main had refactored cmd/client/push.go (and split out push_tui.go). The only conflict was the import block; both slices (from your refactor) and sort (used by skippedSecretFiles) are now in place. go build / go vet / tests green, gofmt clean. The two review items from earlier are still in: the skipped-secrets notice goes to os.Stderr (keeps stdout clean for piping), and the skip list is shared via pruneDir() so shouldSkip and skippedSecretFiles can't drift (locked by TestPruneDir). PTAL 🙏

[somaz94]

Hi @davidchua — friendly ping on this one.

Both review points are addressed: the warning now goes to stderr instead of stdout, and the directory prune list is extracted into a pruneDir() helper with a TestPruneDir covering it. I also rebased onto main on Jul 15, so the branch is currently clean and mergeable.

Happy to make any further changes if something still looks off.