Merged
Conversation
Pull "table", "json", "yaml" into exported OutputTable/OutputJSON/OutputYAML constants so goconst stays green as the package grows. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add S3Credentials struct + LoadS3CredentialsForProfile helper for reading verda_s3_* prefixed keys from the shared credentials INI file, mirroring the existing SharedCredentials pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Carry forward the license-header rollout from chore/update-license-apache2 so subsequent S3 work commits stay focused on feature code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Generate a time-limited HTTPS GET URL for an S3 object without
exposing credentials. Default expiration 1h; accepts any Go duration
via --expires-in. URL goes to stdout for piping; expiration note to
stderr. Structured output (-o json/yaml) emits {url, expires_at}.
Introduces a Presigner interface + presignerBuilder var so tests can
swap in fakes without calling real AWS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to 72d98c7 addressing review findings: - Gate per-file progress lines ("uploaded", "downloaded", "copied" including dryrun previews) behind a non-structured output format so `-o json` / `-o yaml` stdout stays machine-parsable. - Fix CopySource URL encoding: percent-encode bucket and key separately with a literal '/' between them; the old single PathEscape replaced the bucket/key separator with %2F and broke CopyObject for any key containing special characters. - On download error, close and os.Remove the partially-written local file; propagate Close errors instead of dropping them. - Reject recursive-download keys that would escape the destination directory via a new safeJoin helper in transfer.go. - Strengthen TestCp_S3ToS3_SingleFile: use a key with a space and assert the exact CopySource string with ==. - Add TestCp_RecursiveDownload_EscapeAttempt covering the traversal guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
S3 commands are in cmd.go skipCredentialResolution, so Options.Complete()
never runs and AuthOptions.Profile stays "". The three client builders
passed that empty string to LoadS3CredentialsForProfile, which made
ini.GetSection("") return ini.v1's synthetic DEFAULT (uppercase) section
instead of the user's [default] section. Result: "no S3 credentials
configured" right after a successful `verda s3 configure`.
Extract loadCredsFromFactory in helper.go with the same "" -> "default"
fallback that show.go already has, and use it in buildClientDefault,
defaultTransporterBuilder, and defaultPresignerBuilder.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
S3 object storage is not officially available in production yet. Two layers of gating so the command ships with the binary but is invisible to general users: - Registration gate (cmd/cmd.go): s3Enabled() checks VERDA_S3_ENABLED=1|true. Without it, the s3 command tree is not registered and `verda s3 ls` returns "unknown command". - Help-hide (cmd/s3/s3.go): Hidden: true on the parent so even with the env var on, s3 stays out of `verda --help`. The custom usage template in cmd/util/command_groups.go was silently ignoring cobra's Hidden flag; teach it to skip hidden commands so this gate (and any future Hidden command) actually disappears from the top-level help output. At GA, drop s3Enabled() and the if-block in cmd/cmd.go, remove Hidden: true from cmd/s3/s3.go, and clear the pre-release banner from cmd/s3/README.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI security workflow runs `golangci-lint run --no-config -E gosec`, which bypasses .golangci.yaml and thus the test-file exclusions for gosec. Several pre-existing findings in cp_test.go, mv_test.go, and sync_test.go tripped CI: - G306 (WriteFile perms too permissive): 0o644 -> 0o600 - G301 (MkdirAll perms too permissive): 0o755 -> 0o750 - G304 (file inclusion via variable) on os.ReadFile: suppressed with `// #nosec G304 -- dst is under t.TempDir()` (paths are test-controlled) - G602 (slice index out of range) in the hand-written equalStrings helper: deleted the helper and switched callers to slices.Equal Also add `make security` that runs the exact CI command, so this gap between `make pre-commit` and CI does not recur silently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Description
Type of Change
Checklist
make test)make pre-commit)Related Issues
Additional Context