feat: enhance archiving and extraction capabilities with additional compression formats and ownership preservation - #13458
Merged
Merged
Conversation
…ompression formats and ownership preservation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the file archiving/decompression implementation across the agent and frontend, primarily to preserve file owner/group metadata during archive extraction (addressing #13439) and to broaden/align supported compression/extraction behaviors across formats.
Changes:
- Add ownership-preservation support during extraction (SDK and CLI paths), including hardlink handling and safer destination path validation.
- Expand tar-based compression/extraction support to additional formats (e.g., tar.xz / tar.bz2 variants) and improve ZIP ownership metadata handling.
- Improve shared-file password handling in the frontend by removing only the password fragment from the URL hash.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/share/index.vue | Preserves non-password hash state while removing shared-password from URL after applying it. |
| frontend/src/utils/file.ts | Adds helper to remove only the file-share password key from the URL hash. |
| agent/utils/files/zip.go | Adds optional ownership-preserving unzip flags via ExtractWithOptions. |
| agent/utils/files/x7z.go | Adjusts 7z compression flags to better preserve link semantics. |
| agent/utils/files/tar.go | Implements tar-based compression and adds ownership-preserving extraction options for multiple tar compression variants. |
| agent/utils/files/tar_gz.go | Adds ownership-preserving options for tar.gz extraction/decrypt pipelines. |
| agent/utils/files/rar.go | Adds optional ownership-preserving extraction flags and updates rar compression args. |
| agent/utils/files/file_op.go | Major refactor of SDK extraction flow: ownership restore, hardlink/symlink handling, path safety, and CLI↔SDK fallback behavior. |
| agent/utils/files/archiver.go | Extends tar handler registration to more compression types. |
| agent/app/service/file.go | Switches decompression to use ownership-preserving options and improves post-extract copying to preserve hardlinks/modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+27
to
32
| func (t TarGzArchiver) ExtractWithOptions(ctx context.Context, filePath, dstDir, secret string, preserveOwner bool) error { | ||
| if err := os.MkdirAll(dstDir, 0755); err != nil { | ||
| return fmt.Errorf("failed to create destination dir: %w", err) | ||
| } | ||
| cmdMgr := cmd.NewCommandMgr(cmd.WithContext(ctx), cmd.WithIgnoreExist1()) | ||
| cmdMgr := cmd.NewCommandMgr(cmd.WithContext(ctx)) | ||
| if len(secret) != 0 { |
Comment on lines
+1566
to
1575
| shellErr = err | ||
| if cType == TarGz { | ||
| if strings.Contains(err.Error(), "bad decrypt") { | ||
| return buserr.New("ErrBadDecrypt") | ||
| } | ||
| if err := shellArchiver.Extract(ctx, srcFile, dst, "-"); strings.Contains(err.Error(), "bad decrypt") { | ||
| if retryErr := extractWithShellOptions(ctx, shellArchiver, srcFile, dst, "-", options); retryErr == nil { | ||
| return nil | ||
| } else if strings.Contains(retryErr.Error(), "bad decrypt") { | ||
| return buserr.New("ErrBadDecrypt") | ||
| } |
Comment on lines
+41
to
+46
| baseDir := filepath.Dir(sourcePaths[0]) | ||
| args := []string{t.getOptionStr("compress"), dstFile, "-C", baseDir} | ||
| for _, sourcePath := range sourcePaths { | ||
| args = append(args, filepath.Base(sourcePath)) | ||
| } | ||
| return cmd.NewCommandMgr(cmd.WithContext(ctx)).Run(t.Cmd, args...) |
Comment on lines
+1320
to
+1334
| if target == "" && archFile.Open != nil { | ||
| fr, err := archFile.Open() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| data, readErr := io.ReadAll(fr) | ||
| closeErr := fr.Close() | ||
| if readErr != nil { | ||
| return readErr | ||
| } | ||
| if closeErr != nil { | ||
| return closeErr | ||
| } | ||
| target = string(data) | ||
| } |
…ompression formats and ownership preservation
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
agent/utils/files/tar_gz.go:35
TarGzArchiver.ExtractWithOptionspreviously usedcmd.WithIgnoreExist1()(ignore exit code 1). Removing it can cause decompression to fail on common tar warnings where files are still extracted (exit status 1), which would be a behavior regression for users. Consider restoringWithIgnoreExist1()here (and in any tar pipe paths if needed), so warnings don’t abort extraction.
cmdMgr := cmd.NewCommandMgr(cmd.WithContext(ctx))
if len(secret) != 0 {
return runTarGzDecryptToDirWithOptions(cmdMgr, filePath, dstDir, secret, false, preserveOwner)
}
return runTarGzExtractToDirWithOptions(cmdMgr, filePath, dstDir, preserveOwner)
agent/utils/files/file_op.go:1280
- When the archive contains an entry whose name normalizes to "." (e.g. ".", "./"), archiveDestinationPath returns the extraction root (dst). That root path is then appended to
dirs, so the later metadata-restore loop can chmod/chown/chtimes the destination directory itself based on archive metadata. This lets archives unexpectedly modify the destination directory’s ownership/permissions/timestamps and can break the caller’s expectations (and is risky when PreserveOwner=true).
extractionStarted = true
if archFile.FileInfo.IsDir() {
if filePath != filepath.Clean(dst) {
if err := ensureArchiveParent(dst, filePath); err != nil {
return err
…ompression formats and ownership preservation
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
agent/utils/files/tar_gz.go:35
cmd.WithIgnoreExist1()was removed from the tar-based extraction command manager. GNU tar uses exit status 1 for non-fatal warnings (e.g., some metadata/permissions couldn’t be restored), so this change can turn previously successful extractions into hard failures—especially now that--same-owner/--same-permissionsmay be enabled.
cmdMgr := cmd.NewCommandMgr(cmd.WithContext(ctx))
if len(secret) != 0 {
return runTarGzDecryptToDirWithOptions(cmdMgr, filePath, dstDir, secret, false, preserveOwner)
}
return runTarGzExtractToDirWithOptions(cmdMgr, filePath, dstDir, preserveOwner)
agent/utils/files/file_op.go:1260
extractionStartedis only set after the ignore-file check. For.gzhandling, this means a tar-based archive that contains only ignored entries (e.g.__MACOSX/,._*,.DS_Store) will be misclassified as a plain gzip stream and fall back toDecompressGzFile, producing incorrect output. Marking extraction as started even when an entry is ignored prevents this mis-detection.
info := archFile.FileInfo
if isIgnoreFile(archFile.Name()) {
return nil
}
agent/utils/files/tar.go:35
TarArchiver.ExtractWithOptionsenables--same-owner/--same-permissions, which can causetarto exit with status 1 for non-fatal warnings. Since the command helper supports ignoring exit status 1, it’s safer to enable it here (matching other file ops) to avoid failing the whole decompress on recoverable tar warnings.
args := []string{t.getOptionStr("extract"), filePath, "-C", dstDir}
if preserveOwner {
args = append([]string{"--same-owner", "--same-permissions"}, args...)
}
return cmd.NewCommandMgr(cmd.WithContext(ctx)).Run(t.Cmd, args...)
agent/app/service/file.go:540
PreserveOwner: trueis now forced for all decompressions. If the agent process isn’t running as root (or otherwise lacks chown capabilities), ownership restoration will fail and abort extraction/copy. Gating this option by effective UID avoids a regression for non-root deployments while still fixing owner/group loss when running with sufficient privileges.
if err := fo.DecompressWithOptions(t.TaskCtx, c.Path, tempDst, files.CompressType(c.Type), c.Secret, files.DecompressOptions{
PreserveOwner: true,
AllowCLIReextract: true,
}); err != nil {
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
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.
#13439