Skip to content

chore: migrate syft to use mholt/archives instead of anchore fork#22

Merged
l-qing merged 2 commits intoalauda-v1.28.0from
fix/syft-vulnerability
Dec 24, 2025
Merged

chore: migrate syft to use mholt/archives instead of anchore fork#22
l-qing merged 2 commits intoalauda-v1.28.0from
fix/syft-vulnerability

Conversation

@l-qing
Copy link
Copy Markdown

@l-qing l-qing commented Dec 24, 2025

Description

Please include a summary of the changes along with any relevant motivation and context,
or link to an issue where this is explained.

  • Fixes

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (please discuss with the team first; Syft is 1.0 software and we won't accept breaking changes without going to 2.0)
  • Documentation (updates the documentation)
  • Chore (improve the developer experience, fix a test flake, etc, without changing the visible behavior of Syft)
  • Performance (make Syft run faster or use less memory, without changing visible behavior much)

Checklist:

  • I have added unit tests that cover changed behavior
  • I have tested my code in common scenarios and confirmed there are no regressions
  • I have added comments to my code, particularly in hard-to-understand sections

@l-qing
Copy link
Copy Markdown
Author

l-qing commented Dec 24, 2025

cherry-pick from: anchore#4029

@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot Bot commented Dec 24, 2025

✅ No New Issues - Previous Review Still Valid

19 files reviewed | Confidence: 90% | Recommendation: Approve with monitoring

This PR successfully migrates from anchore/archiver to mholt/archives with significant security and architectural improvements.

Key Improvements Confirmed

  • Enhanced Security: Comprehensive symlink attack protection and path traversal prevention
  • Modern API: Migrated to visitor pattern with proper context usage
  • Better Error Handling: More consistent error patterns and logging
  • Code Cleanup: Removed 229 lines of custom ZIP handling in favor of standard library

Security Tests Added

Extensive security test coverage for:

  • Path traversal attacks (../../../etc/passwd)
  • Symlink-based directory escapes
  • Malicious archive extraction scenarios

Dependencies Updated

  • Primary: github.com/mholt/archives (replacing github.com/anchore/archiver/v3)
  • Supporting: Updated nwaples/rardecode/v2, olekukonko/* packages to latest stable versions
  • Removed: Direct golang/snappy dependency (now handled transitively)

Architecture Changes

All archive operations now use the visitor pattern:

archives.Identify(ctx, path, nil)
format.Extract(ctx, reader, visitor)

Status: No New Issues Found

The changes since the previous review are consistent with the original migration plan. No new security concerns, bugs, or architectural issues have been introduced. The comprehensive test suite provides confidence in the changes.

Recommendation

Approve with monitoring - The migration maintains backward compatibility while significantly improving security posture.

@l-qing l-qing force-pushed the fix/syft-vulnerability branch from 7479081 to 048070c Compare December 24, 2025 13:38
…chore#4029)

---------
Signed-off-by: Kudryavcev Nikolay <kydry.nikolau@gmail.com>
Signed-off-by: Christopher Phillips <spiffcs@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@l-qing l-qing force-pushed the fix/syft-vulnerability branch 2 times, most recently from 00edce6 to 64fd8a2 Compare December 24, 2025 14:18
This commit addresses a potential vulnerability where the license
scanner was not properly initialized before use in the Java archive
parser, which could lead to nil pointer dereference.

Additionally, updates test fixtures and assertions to reflect:
- Updated package versions in Rocky Linux (curl-minimal, httpd)
- Refactored deduplication tests to index by package name for
  better resilience to version changes
- Added comprehensive test documentation
@l-qing l-qing force-pushed the fix/syft-vulnerability branch from 64fd8a2 to 3fb3f35 Compare December 24, 2025 14:20
@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot Bot commented Dec 24, 2025

✅ No Issues Found

19 files reviewed | Confidence: 95% | Recommendation: Approve

This PR successfully migrates from github.com/anchore/archiver/v3 to github.com/mholt/archives with significant security and architectural improvements.

Key Improvements Confirmed

  • Enhanced Security: Comprehensive symlink attack protection and path traversal prevention via SafeJoin function
  • Modern API: Migrated to visitor pattern with proper context usage throughout
  • Better Error Handling: More consistent error patterns and logging
  • Code Cleanup: Removed 229 lines of custom ZIP handling in zip_read_closer.go in favor of standard library
  • Dependency Updates: Updated supporting packages (nwaples/rardecode/v2, olekukonko/*) to latest stable versions

Security Tests Added

Extensive security test coverage in zip_file_traversal_test.go:

  • Path traversal attacks (../../../etc/passwd)
  • Symlink-based directory escapes
  • Malicious archive extraction scenarios
  • Forensic analysis of extraction directories
  • Validation of symlink attack prevention

Architecture Changes

All archive operations now use the modern visitor pattern:

// Old API
archiver.ByExtension(path)
archiver.Walk(archivePath, visitor)

// New API  
archives.Identify(ctx, path, nil)
format.Extract(ctx, reader, visitor)

Critical Security Features

  1. Path Traversal Protection: SafeJoin ensures archive entries cannot escape extraction directories
  2. Context Propagation: All functions properly use context.Context for cancellation and timeouts
  3. Symlink Safety: Comprehensive testing confirms symlinks are handled safely
  4. Error Containment: Malicious archives are rejected with clear error messages

Status: No Issues Found

The migration maintains backward compatibility while significantly improving the security posture. All existing functionality is preserved with enhanced protection against archive-based attacks.

Recommendation

Approve - This is a high-quality migration that improves both security and code maintainability.

)

func TestPackageDeduplication(t *testing.T) {
// this test verifies that package deduplication works correctly across layers.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件是从 上游 main 分支检出的代码。


FROM stage1 AS stage2
RUN dnf update -y curl-minimal-7.76.1-31.el9_6.1
RUN dnf update -y curl-minimal-7.76.1-34.el9
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件是 AI 帮忙修改的,原来的版本不存在了。

@l-qing l-qing merged commit efec31b into alauda-v1.28.0 Dec 24, 2025
10 checks passed
@l-qing l-qing deleted the fix/syft-vulnerability branch December 24, 2025 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants