feat(cooldown): respect trusted_packages in dependency cooldown#342
Merged
Conversation
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 54.13% 54.18% +0.04%
==========================================
Files 180 180
Lines 12844 12903 +59
==========================================
+ Hits 6953 6991 +38
- Misses 5225 5248 +23
+ Partials 666 664 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
abhisek
reviewed
Jun 15, 2026
Sahilb315
added a commit
that referenced
this pull request
Jun 15, 2026
Address review feedback on #342: - Restore cooldownSkip to a pure single-list function (SRP); the merge into trusted_packages now happens in a separate mergeCooldownSkip step, driven by the exported CooldownSkip wrapper. - Extend CooldownSkipInfo with a CooldownSkipReason (TrustedPackage / CooldownSkipList) on both SkipAll and per-version entries, so callers can tell apart the broad waiver from the cooldown-only one. When both lists match the same package, trusted_packages wins. - Add audit.LogCooldownSkipped and emit it from the npm and PyPI interceptors on the SkipAll path, alongside the existing info log, carrying the source list as the reason.
abhisek
reviewed
Jun 16, 2026
Trusted packages are now treated as a superset waiver that bypasses every PMG control (malware analysis, cooldown, and any future controls). A globally trusted package is automatically exempt from the cooldown window and no longer needs a duplicate entry in dependency_cooldown.skip. The skip list remains the narrower, cooldown-only waiver for packages that must bypass the cooldown wait but still be malware-scanned.
Address review feedback on #342: - Restore cooldownSkip to a pure single-list function (SRP); the merge into trusted_packages now happens in a separate mergeCooldownSkip step, driven by the exported CooldownSkip wrapper. - Extend CooldownSkipInfo with a CooldownSkipReason (TrustedPackage / CooldownSkipList) on both SkipAll and per-version entries, so callers can tell apart the broad waiver from the cooldown-only one. When both lists match the same package, trusted_packages wins. - Add audit.LogCooldownSkipped and emit it from the npm and PyPI interceptors on the SkipAll path, alongside the existing info log, carrying the source list as the reason.
Address further review feedback: - Drop the separate mergeCooldownSkip helper; cooldownSkip now writes into a shared *CooldownSkipInfo and is called twice from CooldownSkip (cooldown skip list first, trusted_packages on top so trusted entries override the reason on overlap). - Audit log every exemption, not just SkipAll: a new auditCooldownSkip helper in proxy/interceptors/cooldown.go emits one event per match (package-wide or per-version), each tagged with its source list. LogCooldownSkipped gains a version argument for the per-version case. - Cover the trusted_packages reason path in TestCooldownSkip.
auditCooldownSkip now only emits EventTypeCooldownSkipped for entries that came from dependency_cooldown.skip. Trusted-package exemptions already get an EventTypeInstallTrustedAllowed event at tarball-download time (proxy/interceptors/base_registry.go), so emitting a cooldown event for them too would double-count the same waiver.
Sahilb315
force-pushed
the
dep-cooldown-respect-trusted-pkgs
branch
from
b3059e7 to
f3beb49
Compare
vet Summary ReportThis report is generated by vet Policy Checks
Malicious Package AnalysisMalicious package analysis was performed using SafeDep Cloud API Malicious Package Analysis Report
Changed PackagesChanged Packages
|
abhisek
reviewed
Jun 18, 2026
…uit on trusted SkipAll Address PR review feedback: - Rename cooldownSkip to collectCooldownSkip and return CooldownSkipInfo instead of mutating an input pointer. - Add mergeCooldownSkip to combine per-list results with trusted_packages taking precedence on overlap. - CooldownSkip now consults trusted_packages first and returns immediately on a package-wide trusted exemption (DC skip list cannot add anything). - Extend tests to cover disjoint pinned entries across both lists and the case where DC version-less subsumes a trusted pinned entry.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates PMG’s dependency cooldown waiver model so the global trusted_packages list acts as a superset exemption (bypassing cooldown and other controls), while dependency_cooldown.skip remains a cooldown-only exemption. It also adds audit visibility for cooldown exemptions and updates docs/templates to match the new semantics.
Changes:
- Extend cooldown skip resolution to union
trusted_packages+dependency_cooldown.skip, tagging the exemption source. - Emit a new audit event for cooldown exemptions coming specifically from
dependency_cooldown.skip, and translate it to a ControlTower package decision action. - Update documentation, config template/comments, and tests to reflect the new waiver model.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| proxy/interceptors/pypi_registry.go | Uses updated cooldown skip info (version set) and audits cooldown-skip exemptions. |
| proxy/interceptors/npm_registry.go | Uses updated cooldown skip info (version set) and audits cooldown-skip exemptions. |
| proxy/interceptors/cooldown.go | Adds auditCooldownSkip helper to log/audit cooldown exemptions sourced from dependency_cooldown.skip. |
| internal/audit/event.go | Introduces new audit event type dependency_cooldown_skipped. |
| internal/audit/cloud_translate.go | Translates cooldown-skipped audit events into ControlTower package decision actions. |
| internal/audit/cloud_translate_test.go | Adjusts unsupported-event coverage list for translation tests. |
| internal/audit/audit.go | Adds LogCooldownSkipped audit API for cooldown exemption events. |
| config/trusted.go | Implements union behavior and source-tagging for cooldown exemptions across trusted and cooldown-skip lists. |
| config/cooldown_skip_test.go | Reworks tests to validate trusted-packages superset behavior and reason tagging. |
| config/config.template.yml | Updates template guidance to describe trusted vs per-control cooldown skip behavior. |
| config/config.go | Updates DependencyCooldownConfig.Skip doc comment to reflect new model. |
| docs/trusted-packages.md | Documents trusted packages as a global waiver (including cooldown). |
| docs/dependency-cooldown.md | Updates cooldown docs to describe trusted_packages as superset waiver and skip list as per-control. |
| go.mod | Bumps safedep protocolbuffers module version; adds connectrpc module indirectly. |
| go.sum | Adds sums for updated/added generated modules. |
| go.work.sum | Adds sums for new/updated workspace dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
abhisek
reviewed
Jun 19, 2026
abhisek
reviewed
Jun 19, 2026
…rsion Backend rejects PackageVersion messages without a version, and audit logs should reflect the runtime fact (a specific version was skipped) rather than the config rule. Move the audit emission from metadata-request handling to download-request handling, where the concrete version is known, and require version in LogCooldownSkipped.
abhisek
reviewed
Jun 19, 2026
Registry interceptors no longer compute CooldownSkip or branch on SkipAll; they just call HandleMetadataRequest. The npm and pypi cooldown handlers own the skip lookup, the package-wide exemption short-circuit, and (for pypi) the canonical-name denormalization. Also align LogCooldownSkipped with other LogXxx signatures by taking *packagev1.PackageVersion.
…pts for trusted and DC skip packages
abhisek
previously approved these changes
Jun 21, 2026
abhisek
approved these changes
Jun 21, 2026
KunalSin9h
approved these changes
Jun 21, 2026
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.
Summary
trusted_packageslist as a superset waiver: a trusted package now bypasses every PMG control — malware analysis, dependency cooldown, and any future controls — without needing a duplicate entry independency_cooldown.skip.dependency_cooldown.skipkeeps its existing role as the narrower, cooldown-only waiver (still subject to malware analysis).docs/dependency-cooldown.md,docs/trusted-packages.md, the config template, and theDependencyCooldownConfig.Skipdoc comment to reflect the new model.TestCooldownSkipRespectsTrustedPackagescovering the union behavior for both version-less and version-pinned trusted entries.