Skip to content

Conversation

@piquark6046
Copy link
Member

No description provided.

@piquark6046 piquark6046 marked this pull request as ready for review January 7, 2026 13:56
Copilot AI review requested due to automatic review settings January 7, 2026 13:56
@piquark6046 piquark6046 merged commit bd22752 into main Jan 7, 2026
11 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reverts the build approach from TypeScript AST manipulation to direct banner injection. Instead of dynamically modifying the source code to inject domains, the domains are now injected directly into the userscript metadata header using @match directives.

  • Removed ts-morph dependency and related AST manipulation code
  • Replaced dynamic TypeScript source file generation with direct banner template processing
  • Simplified the build pipeline by eliminating temporary source file creation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
sources/banner.txt Replaced hardcoded wildcard @match patterns with a %%DOMAIN_INJECTION%% placeholder for dynamic domain injection
package.json Removed ts-morph dependency as it's no longer needed
builder.ts Refactored to inject domains directly into the banner header instead of manipulating TypeScript source files; removed temporary file generation and cleanup logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ConvertedHeader += Line.replaceAll('%%VERSION_VALUE%%', Version) + '\n'
} else if (Line.includes('%%NAME%%')) {
ConvertedHeader += Line.replaceAll('%%NAME%%', BuildType === 'production' ? 'tinyShield' : 'tinyShield (Development)') + '\n'
} else if (Line === '%%DOMAIN_INJECTION%%') {
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The condition Line === '%%DOMAIN_INJECTION%%' uses strict equality which will fail if the line contains any leading or trailing whitespace. Since the previous conditions use .includes(), this inconsistency could cause the placeholder to not be replaced if the banner.txt file has any whitespace around the placeholder. Consider using .includes('%%DOMAIN_INJECTION%%') for consistency or .trim() === '%%DOMAIN_INJECTION%%' to handle whitespace.

Suggested change
} else if (Line === '%%DOMAIN_INJECTION%%') {
} else if (Line.trim() === '%%DOMAIN_INJECTION%%') {

Copilot uses AI. Check for mistakes.
//
// @match https://*/*
// @match http://*/*
%%DOMAIN_INJECTION%%
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The %%DOMAIN_INJECTION%% placeholder is expanded at build time into @match directives using domains fetched from https://info.ad-shield.io/sellers.json in builder.ts, but those domain strings are only validated with new URL and then interpolated directly into the userscript header. Because this remote JSON is third-party and the domain field can contain characters such as newlines, a compromised or malicious endpoint can inject additional header directives (for example @require or overly broad @match rules) into the built script, effectively turning the sellers.json feed into a code-supply source for all users. To reduce this risk, strictly sanitize the remote domain values (e.g., restrict to hostname characters and derive the value from new URL(...).hostname or similar) before using them in @match lines so they cannot break out of the intended pattern.

Copilot uses AI. Check for mistakes.
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.

2 participants