-
Notifications
You must be signed in to change notification settings - Fork 1.5k
ccleaner: remove installed cruft #16502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughUpdated Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Uninstaller
participant PowerShell
participant gsudo
note over Uninstaller,PowerShell #DDEEFF: post_uninstall flow (elevated task removal)
User->>Uninstaller: trigger uninstall
Uninstaller->>PowerShell: execute post_uninstall script
PowerShell->>gsudo: Invoke Unregister-ScheduledTask & remove .lnk
gsudo-->>PowerShell: command result (success / continue on error)
PowerShell-->>Uninstaller: post_uninstall completed
Uninstaller-->>User: uninstall finished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with Invalid manifests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/ccleaner.json(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull Requests
bucket/ccleaner.json
[error] 1-1: Conversion failed. Invalid JSON in manifest. PR contains invalid manifests: ccleaner.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (2)
bucket/ccleaner.json (2)
37-38: Verify the scheduled task name matches CCleaner's actual registration.The implementation assumes CCleaner registers a scheduled task named
CCleanerCrashReporting. Confirm this task name is correct and persists across CCleaner versions.
6-6: gsudo dependency verified as valid Scoop package.gsudo is available through Scoop and can be installed with
scoop install gsudo. It is listed in the main Scoop bucket as a recommended tool for executing commands with administrative privileges. The dependency is appropriate for executing the admin-privilegedUnregister-ScheduledTaskcommand as documented.
c6de7b2 to
10700f5
Compare
10700f5 to
500fe85
Compare
|
Before addressing the issues mentioned in this PR, we may need to first resolve the problem of the current manifest being unable to auto-update (see the ccleaner section in result.txt for details). Otherwise, the |
|
Very quick looking around seems like CCleaner changed ownership from Piriform to the Avest/NortonLifeLock corp. It seems they have
I formulated a workaround, but mostly for the mental exercise. I believe CCleaner should be removed from the bucket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/ccleaner.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T09:37:06.093Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16328
File: bucket/winutil.json:0-0
Timestamp: 2025-10-13T09:37:06.093Z
Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.
Applied to files:
bucket/ccleaner.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (3)
bucket/ccleaner.json (3)
32-39: Verify post_uninstall correctly handles privileged task unregistration.The post_uninstall now includes
gsudo Unregister-ScheduledTaskto remove theCCleanerCrashReportingtask. Verify that:
- The
-Confirm:0 -ErrorAction Continueflags correctly suppress confirmation and handle non-existent tasks gracefully (no error if task doesn't exist).- gsudo is correctly invoked and properly depends on the
gsudopackage.- The task name
CCleanerCrashReportingmatches the actual scheduled task name created by CCleaner.This addresses the PR's core objective of removing leftover scheduled tasks.
40-43: Checkver regex extraction verified, but autoupdate hash verification is broken.The checkver regex correctly extracts version components:
ccsetup639.zip→ major=6, minor=39, producing version6.39. Scoop's autoupdate correctly splits version6.39on dots to populate$majorVersion=6and$minorVersion=39, making the autoupdate URL template resolve correctly tohttps://download.ccleaner.com/portable/ccsetup639.zip(a valid URI).However, the
autoupdatesection has a critical flaw: thehash.urlfield is empty. This prevents Scoop from verifying the downloaded file's integrity during autoupdate, breaking the autoupdate mechanism entirely. Provide a valid hash URL (or remove the emptyhashobject if hash verification is not supported for this URL).⛔ Skipped due to learnings
Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16378 File: bucket/compactgui.json:20-22 Timestamp: 2025-10-19T13:58:23.389Z Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.Learnt from: Gitoffthelawn Repo: ScoopInstaller/Extras PR: 16106 File: bucket/czkawka-gui.json:25-25 Timestamp: 2025-09-05T09:41:52.653Z Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16349 File: bucket/fvim.json:45-49 Timestamp: 2025-10-16T15:59:21.258Z Learning: In Scoop manifests, the `autoupdate.url` and `autoupdate.architecture.<arch>.url` fields must be valid URIs according to the JSON schema (defined with `"format": "uri"`). Variables like `$matchUrlx64` that contain only path segments must be combined with a base URL (e.g., `https://github.com/.../releases/download/`) to form a complete valid URI.Learnt from: Gitoffthelawn Repo: ScoopInstaller/Extras PR: 16106 File: bucket/czkawka-gui.json:25-25 Timestamp: 2025-09-05T09:41:52.653Z Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
2-2: The version truncation is intentional and correctly addresses a schema mismatch; the review comment's framing is inaccurate.The version was intentionally changed from "6.39.11548" to "6.39" as part of a "workaround wonky schema and Piriform mismatch", not as a side-effect. Scoop compares dot-separated numeric fields left-to-right; 6.39 (treated as 6.39.0) will correctly sort as less than 6.39.11548, so version comparison and upgrades will function properly—there is no downgrade or compatibility risk.
However, there is a real trade-off: CCleaner frequently ships security and hotfixes by incrementing only the build number while keeping major.minor stable, and the new URL-based checkver strategy extracts version from the download filename (ccsetup639.zip → 6.39), which cannot detect build-only increments. This is a coverage limitation, not a version-comparison defect, and was an accepted trade-off to resolve the upstream schema inconsistency.
Likely an incorrect or invalid review comment.
| "hash": { | ||
| "url": "" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty hash URL disables integrity verification in autoupdate.
The autoupdate.hash.url field is set to an empty string, which prevents the autoupdate mechanism from verifying the integrity of downloaded files. This is a security and reliability risk.
Provide a valid hash URL or find an alternative hash verification method:
- "hash": {
- "url": ""
- }
+ "hash": {
+ "url": "https://download.ccleaner.com/hashes.txt"
+ }You may need to verify the actual hash source URL for CCleaner's portable builds.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bucket/ccleaner.json around lines 47 to 49, the autoupdate.hash.url is an
empty string which disables integrity verification; replace the empty value with
the correct HTTPS URL that returns the hash for the CCleaner portable build (or
point to a trusted checksum file/endpoint for the exact build/version used), or
if no remote hash exists, remove the empty url and implement an alternative
verification (embed a pinned hash or fetch and validate a signed checksum) so
autoupdate can perform integrity checks instead of skipping them.
I believe we should contact the official team first to confirm whether they plan to continue providing portable builds in future releases. If they do not intend to maintain them, then CCleaner should first be moved to the Nonportable bucket, followed by initiating the deprecation process for the entry in the Extras bucket. Alternatively, we could consider using |
Unregister-ScheduledTaskto "post_uninstall"CCleaner (a.k.a. crap-cleaner) is now registering an admin privileged scheduled task that persist
scoop uninstall, a.k.a. crap.Ironic
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit