- Handle case where basepath equals targpath by returning "."
- Ensure basepath is treated as directory with trailing slash to avoid false prefix matches
- Fix linter issue by using += operator
Addresses code review feedback.
Co-authored-by: rchincha <45800463+rchincha@users.noreply.github.com>
Add pathRel helper method to ImageStore that handles path operations correctly for both local and non-local storage drivers. For S3 and other non-local drivers, use forward slashes consistently to avoid Windows path separator issues where filepath.Rel fails when paths use forward slashes but Windows expects backslashes.
This fixes the memory exhaustion and high CPU usage issue on Windows when using S3 storage, which was caused by infinite loops in GetRepositories and related methods.
Co-authored-by: rchincha <45800463+rchincha@users.noreply.github.com>
Add support for configurable identity attributes in mTLS authentication,
allowing identity extraction from CommonName, Subject DN, Email SAN,
URI SAN, or DNSName SAN with fallback chain support. Includes regex
pattern matching for URI SANs (e.g., SPIFFE workload IDs).
- Add MTLSConfig with identity attributes, URISANPattern, and index fields
- Implement extractMTLSIdentity with fallback chain logic
- Move the mtls tests in the api package to pkg/api/mtls_test.go
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
* fix: prevent nil pointer dereference in RemoveImageFromRepoMeta
This commit fixes a critical bug where RemoveImageFromRepoMeta crashes with
a nil pointer dereference during retention policy execution and GC operations.
Root Cause:
The function was accessing blob metadata without checking if it exists first.
During GC/retention operations, the metadata database might have stale
references to blobs that no longer exist, causing runtime panics.
Changes:
- Added nil check for descriptorBlobInfo before accessing LastUpdated field
- Added nil check for blobInfo before dereferencing Size, Vendors, Platforms, and SubBlobs
- Made the function consistent with recalculateAggregateFields which already had these checks
Impact:
- Fixes crashes during retention policy execution
- Fixes crashes during GC manifest removal
- Fixes image deletion failures via API
- Eliminates need for dryRun: true workaround in retention config
The fix gracefully handles missing blob metadata by skipping those entries
instead of crashing.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: add comprehensive tests for RemoveImageFromRepoMeta nil checks
Add test coverage for the nil pointer dereference fixes in RemoveImageFromRepoMeta.
These tests ensure the function handles missing blob metadata gracefully during
GC and retention operations.
Test cases:
- Handle nil blob info for descriptor digest (line 280 check)
- Handle nil blob info in queue traversal (line 297 check)
- Verify correct behavior with valid blob info
- Handle empty tags edge case
- Skip tags with empty digest
Coverage: RemoveImageFromRepoMeta now has 100% test coverage
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: fix RemoveImageFromRepoMeta tests to match actual usage
Address review feedback:
- Delete tag from repoMeta.Tags before calling RemoveImageFromRepoMeta
- Fix blob count expectations after tag removal
- Add assertion to verify tag was removed from metadata
- Update comments to clarify expected behavior
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: add tag removal assertion to second test case
Add missing assertion to verify tag1 was removed from resultMeta.Tags
in the 'should handle nil blob info in queue traversal' test.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* refactor: improve nil blob handling documentation and test coverage
Address Copilot review feedback:
- Expand comment at line 278 to explain implications of skipping tags
with missing blob info, clarifying that metadata inconsistency is
acceptable in GC/cleanup scenarios
- Revise 'should handle nil blob info for descriptor digest' test to
cover more realistic scenario: remove tag1 while tag2 has missing
blob info, demonstrating graceful handling of data inconsistencies
in remaining tags during removal operations
All tests pass with 49 total assertions.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* fix: prevent nil pointer in GetCandidates when statistics missing
Add defensive check in pkg/retention/candidate.go to handle cases where
a tag exists in repoMeta.Tags but has no corresponding entry in
repoMeta.Statistics. This prevents incorrect retention decisions based
on zero-value timestamps.
Changes:
- Check statistics existence before creating candidates
- Skip tags with missing statistics (retained by GetRetainedTagsFromMetaDB)
- Improve performance from O(n*m) to O(n) by using direct map lookup
- Add comprehensive test coverage for missing statistics scenarios
This addresses the concern raised in PR #3658 about metadata
inconsistencies due to non-transactional writes to blob store and metaDB.
Related: #3658
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
* test: achieve 100% coverage for RemoveImageFromRepoMeta nil checks
Enhance test coverage for RemoveImageFromRepoMeta to address codecov failures
by adding comprehensive test cases that exercise all code paths including nil
pointer checks and continue statements.
Changes:
- Enhanced 'nil blob info for descriptor digest' test to verify processing
continues with other valid tags after skipping nil entries
- Enhanced 'nil blob info in queue traversal' test to handle mixed valid/nil
sub-blobs and verify correct processing continuation
- Added 'multiple nil blobs in deeply nested structure' test to cover complex
scenarios with multiple missing blobs at various nesting levels
- Enhanced 'skip tags with empty digest' test to verify processing continues
with valid tags after skipping empty digest entries
- Added 'combined edge cases' test to verify all edge cases work together:
empty digest, nil descriptor blob, and nil queue blob
Coverage Results:
- RemoveImageFromRepoMeta: 100.0% line coverage (was 87.50%)
- All 7 test scenarios pass with 75 total assertions
- All nil check code paths fully exercised
- All continue statement behaviors validated
Fixes codecov/patch failure on PR #3658 where 2 lines were missing coverage.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
---------
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
- Refactored HTTP client from global cache to struct-based approach (global state was shared between tests, including what certificates to use)
- Enhanced pkg/test/tls to support ECDSA and ED25519 key types
- Replaced static certificate files with dynamic generation in golang tests
- Fixed test cleanup issues and improved resource management
This eliminates dependency on external cert generation scripts and
improves test maintainability.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
* feat: support mTLS-only authn/authz with AccessControl and allow combining mTLS with other auth mechanisms
Signed-off-by: Ivan Arkhipov <me@endevir.ru>
* refactor: improve authentication logic and TLS certificate generation
- Fix mTLS authentication to use only leaf certificate instead of iterating
through all certificates in the chain
- Reject Authorization headers when corresponding auth method is disabled,
regardless of mTLS status (security improvement)
- Simplify authentication switch statement ordering and logic
- Move ErrUserDataNotFound error handling into sessionAuthn method
- Refactor TLS certificate generation to use Options pattern with
CertificateOptions struct for better extensibility
- Consolidate duplicate certificate generation code into helper functions
(generateCertificate, parseCA, initializeTemplate, applyOptions)
- Rename certificate generation functions for clarity:
- GenerateCertWithCN -> GenerateClientCert
- GenerateSelfSignedCertWithCN -> GenerateClientSelfSignedCert
- Add support for SAN settings including email addresses in certificates
- Update tests to reflect new authentication behavior and certificate API
This commit improves both the security posture (rejecting disabled auth
methods) and code maintainability (consolidated certificate generation).
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
* fix: guard against multiple Authorization headers
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
---------
Signed-off-by: Ivan Arkhipov <me@endevir.ru>
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Co-authored-by: Ivan Arkhipov <me@endevir.ru>
See: https://github.com/project-zot/zot/issues/3560#issuecomment-3594856118
What happens is:
- syncRef skips the image ("skipping image because it's already synced")
- syncReferrers doesn't sync anything
- CommitAll is still called even though nothing was synced
- The temp directory exists but is empty (no index.json, no blobs)
- CommitAll fails because index.json is missing
Let's ensure we properly check for errors, and skip the log messages if some of the cases.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Replace MakeTempFile usage with MakeTempFilePath and MakeTempFileWithContent
helpers that automatically handle file lifecycle. This prevents resource
leaks by ensuring temporary files are properly closed.
Shoudld also make the tests easier to read.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Add validation to reject configuration files where stores and substores
of the same storage type (local or S3) have root directories that are
nested within each other or identical. Stores of different types (local
vs S3) are allowed to share the same root directory since they use
different storage backends.
The validation:
- Checks all stores (default + substores) for path conflicts
- Only compares stores of the same storage type
- Reports clear error messages indicating which stores conflict and why
Add comprehensive tests covering:
- Same storage types with identical/nested paths (rejected)
- Different storage types with same/nested paths (allowed)
- Various combinations of default store and substores
Fixes issues where nested or identical root directories could cause
data corruption or routing conflicts.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
fix: error handling: return nil explicitly on successful completion
Several functions in pkg/meta/redis/redis.go were returning 'err' at the
end of successful execution paths, which could lead to incorrect error
handling when 'err' was overwritten in loops or conditionals.
Changed the following functions to return nil explicitly when all
operations succeed:
- SearchRepos: return nil instead of err after successful loop
- SearchTags: return nil instead of err after successful loop
- GetRepoMeta: return nil instead of err after successful operations
- GetImageMeta: return nil instead of err after successful operations
- GetReferrersInfo: return nil instead of err after successful loop
This ensures that when functions complete successfully, they explicitly
return nil rather than relying on the last value of err, which may have
been overwritten during execution. This fixes TestRedisUnreachable which
was failing because SearchRepos was incorrectly returning nil error when
Redis was unreachable.
See failure in: https://github.com/project-zot/zot/actions/runs/19729927463/job/56528529923?pr=3599
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
fix(meta): handle cases where repositories when substores are nested
Note this is a case of bad configuration: having multiple stores
in the same tree structure. Guard against it in parse.go.
Fix getAllRepos to prevent duplicate repositories in metaDB when substore
directories are nested under the default store root directory.
The fix processes substores first, then the default store, using a
map-based deduplication approach to skip repositories that have already
been added. This ensures that when both the default store and substores
contain repositories with the same name (e.g., when a substore is nested
within the default store), only one instance is added to the repository
list.
Add test TestNoDuplicateReposWithSubstoresAndNestedRepoNames to verify
the deduplication logic works correctly with nested substores.
Also update the other tests to avoid these issues in the future
this is not a vali configuration.
This is not the intended use case for substores, and it may have caused:
https://github.com/project-zot/zot/actions/runs/19665302669/job/56320640980
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
It requires the encoding/json/jsontext
package which is only available when the goexperiment.jsonv2 build
tag is enabled. This was causing build constraint errors during
tests and builds.
Changes:
- Add GOEXPERIMENT=jsonv2 to Makefile export and all go build/test
commands that use env (since env creates a fresh environment)
- Add GOEXPERIMENT=jsonv2 to GitHub workflows that use direct go
commands (workflows using make inherit it from Makefile)
Fix other dependabot alerts.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Require blob files to follow standard OCI image layout:
rootDir/repo/blobs/algorithm/digest
- Validate grandparent directory is ImageBlobsDir
- Validate parent directory is valid digest algorithm
- Update tests to use standard OCI structure
- Add blobPath() helper to reduce duplication and fix linting
This should reduce the number of uneeded digest computations
if other non-oci specific files are present in the layout.
Fix also a race condition when picking ports in monitoring tests.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
When GetRepoLastUpdated fails (e.g., redis: nil), it returns time.Time{}
which gets converted to &time.Time{} (not nil). The existing nil checks
in sorting functions didn't account for zero time values, which should
also be treated as "oldest" (sorted last in descending order).
This commit:
- Adds zero time checks in addition to existing nil checks
- Treats both nil and zero time values as "oldest"
- Adds comprehensive tests for zero time edge cases
Affects ImgSortByUpdateTime, RepoSortByUpdateTime, and GetExpandedRepoInfo
sort functions.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit modernizes code across multiple packages by:
- Using Go 1.18+ features (slices.IndexFunc, strings.Cut)
- Pre-allocating slices and maps with known capacity
- Consolidating defensive checks and improving code clarity
- Fixing test data and build tag issues
CLI client improvements:
- Pre-allocate slices in search functions and service methods
- Replace strings.Split with strings.Cut for username:password parsing
- Use range-based iteration instead of manual index loops
Search extension optimizations:
- Cache sort functions in pagination modules
- Pre-allocate page buffers and maps
- Consolidate defensive checks in filterBaseImages/filterDerivedImages
- Fix image bas and derived logic allowing out of sequence layers for base images
- Fix image pagination reporting images groupped by repos when sorted by update time
- Remove duplicate resolver_test.go file
Monitoring extension:
- Replace manual loops with slices.IndexFunc
- Pre-allocate bucketsFloat2String map
Sync extension:
- Pre-allocate slice in parseRegistryURLs
Test utilities:
- Fix build tags in oci_layout.go
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
- Return CommitAll errors instead of ignoring them
- Skip ErrRepoNotFound from temp sync dirs to allow other tags to sync
- Each tag uses separate temp directory, so failures are isolated
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
feat: Sync images with a background context
This means syncs/pulls will not be cancelled anymore when the requesting client disconnects.
The timeout used can be configured per registry
Signed-off-by: Lars Francke <git@lars-francke.de>
fix: make sure metadb statistics are initialized on image download, and minor metadb fixes for Docker v2 manifest compatibility
Looking into potential causes of https://github.com/project-zot/zot/issues/3163
1. One possible reason is the statistics were not properly initialized in the first place because of (unknown and/or unavoidable) errors on image push.
To workaround this add logic to initialize the statistics on the call to download them.
2. Some images have the download statistics while others dont, one cause could be a bug in the logic handling manifest mediatypes in the search extension.
Add compatibility checks for Docker v2 manifest types in metadb convert functions, and more tests for covering the Docker mediatype use case.
Side fixes:
- Ensure PushedBy Statistics entries are properly initialized in SetRepoReference
- Fix and issue in the image upload test functions, they were uploading docker images with oci mediatypes in call headers
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
Old message example:
```
{"time":"2025-11-17T22:29:29.434073547Z","level":"debug","message":"image is not scannable","digest":"sha256:99155f2825186175c7f7e4df4f77d5d0ba68a018603afd9c014063d4ca11c0d6","mediaType":"application/vnd.oci.image.manifest.v1+json","error":"image 'nginx@sha256:99155f2825186175c7f7e4df4f77d5d0ba68a018603afd9c014063d4ca11c0d6' scanning is not supported for given image media type","caller":"zotregistry.dev/zot/v2/pkg/extensions/search/cve/cve.go:513","func":"zotregistry.dev/zot/v2/pkg/extensions/search/cve.BaseCveInfo.GetCVESummaryForImageMedia","goroutine":1246}
```
New message example:
```
{"time":"2025-11-17T22:25:10.33273421Z","level":"debug","message":"image is not scannable","digest":"sha256:99155f2825186175c7f7e4df4f77d5d0ba68a018603afd9c014063d4ca11c0d6","mediaType":"application/vnd.oci.image.manifest.v1+json","error":"image 'nginx@sha256:99155f2825186175c7f7e4df4f77d5d0ba68a018603afd9c014063d4ca11c0d6' scanning is not supported for given media type: layer media type 'application/octet-stream'","caller":"zotregistry.dev/zot/v2/pkg/extensions/search/cve/cve.go:513","func":"zotregistry.dev/zot/v2/pkg/extensions/search/cve.BaseCveInfo.GetCVESummaryForImageMedia","goroutine":1316}
```
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
GC and scrub should not stop if a manifest or index is missing from storage.
Other similar changes are also included.
WRT metadb, the missing manifests cannot be added, and the results returned from metadb
do not include the descriptors for these manifests.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
See: https://github.com/project-zot/zot/issues/2506
Note we are not loosing anything functionality-wise by making this change.
Initially we considered the tags are in the annotations present in the referrers
but the only annotations we set on referrers are the ones inside the manifests themselves,
not the ones in the manifest descriptors, so the tags were not presetn anyway.
Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
feat: add support for sha256 and sha512 htpasswd formats
Fixes issue #3495
We currently support only bcrypt htpasswd hashes, however bcrypt is not
FIPS-140 approved since it uses Blowfish.
This PR adds support for sha256 and sha512 formats and enforces that
bcrypt be disabled when fips140 mode is enabled.
Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
- Use custom authURL/tokenURL from config instead of hardcoded github.com endpoints
- Properly configure GitHub Enterprise API base URL from auth endpoints
Fixes OAuth2 authentication with GitHub Enterprise Server and other
self-hosted OAuth2 providers.
Signed-off-by: Mathias Bogaert <mathias.bogaert@gmail.com>
The "func" field in log output is incorrect in some cases and
showing internal logging methods instead of the actual calling
function. This is happening because the caller information is
being captured statically (5 call frames) which can be different
for different call patterns.
Changes:
* Move the caller capture to the event creation
* Use deterministic skip (3 frames) for event creation
* Add test cases to verify that the caller is captured correctly
Note: tests needed to written outside convey to avoid call insertion
Signed-off-by: Ravi Chamarthy <ravi@chamarthy.dev>
copilot-swe-agent[bot]