Skip to content

Conversation

@ascorbic
Copy link
Owner

@ascorbic ascorbic commented Aug 9, 2025

No description provided.

ascorbic and others added 2 commits August 9, 2025 12:59
…n permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@changeset-bot
Copy link

changeset-bot bot commented Aug 9, 2025

⚠️ No Changeset found

Latest commit: 7d6c604

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ascorbic ascorbic requested a review from Copilot August 17, 2025 17:28

This comment was marked as outdated.

@ascorbic ascorbic requested a review from Copilot August 24, 2025 09:42
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 adds a comprehensive cache handlers package that provides unified cache management functionality for Web API environments. The package includes cache reading/writing, invalidation operations, conditional request support, and stale-while-revalidate (SWR) handling.

Key changes:

  • Complete cache handling implementation with read/write operations and invalidation support
  • Unified configuration system with TypeScript tooling setup
  • Comprehensive test coverage across Node.js, Deno, and workerd environments

Reviewed Changes

Copilot reviewed 71 out of 76 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tsdown.config.ts Root build configuration for TypeScript compilation
tsconfig.base.json Updated base TypeScript configuration with modern settings
pnpm-workspace.yaml Added demos directory to workspace
packages/cdn-cache-control/package.json Fixed repository URL format
packages/cache-handlers/ Complete cache handlers implementation with source files, tests, and configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +387 to +388
if (tags.length > 100) {
throw new Error("Too many cache tags (max 100)");
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The magic number 100 for maximum cache tags should be defined as a named constant to improve maintainability and make it configurable.

Suggested change
if (tags.length > 100) {
throw new Error("Too many cache tags (max 100)");
if (tags.length > MAX_CACHE_TAGS) {
throw new Error(`Too many cache tags (max ${MAX_CACHE_TAGS})`);

Copilot uses AI. Check for mistakes.
Comment on lines +359 to +360
if (tag.length > 100) {
throw new Error("Cache tag too long (max 100 characters)");
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The magic number 100 for maximum tag length should be defined as a named constant to improve maintainability and allow for easier configuration changes.

Suggested change
if (tag.length > 100) {
throw new Error("Cache tag too long (max 100 characters)");
if (tag.length > MAX_CACHE_TAG_LENGTH) {
throw new Error(`Cache tag too long (max ${MAX_CACHE_TAG_LENGTH} characters)`);

Copilot uses AI. Check for mistakes.
Comment on lines +363 to +380
// Remove ALL control characters (0-31) and DEL (127) except space (32)
// deno-lint-ignore no-control-regex
const sanitized = tag.replace(/[\x00-\x1F\x7F]/g, "").trim();

if (sanitized.length === 0) {
throw new Error("Cache tag cannot be empty after sanitization");
}

// Validate against common injection patterns
if (
sanitized.includes("<") ||
sanitized.includes(">") ||
sanitized.includes('"')
) {
throw new Error('Cache tag contains invalid characters (<, >, ")');
}

return sanitized;
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The regex removes control characters but the sanitization approach could be more comprehensive. Consider documenting what characters are allowed and why, as this affects cache security.

Suggested change
// Remove ALL control characters (0-31) and DEL (127) except space (32)
// deno-lint-ignore no-control-regex
const sanitized = tag.replace(/[\x00-\x1F\x7F]/g, "").trim();
if (sanitized.length === 0) {
throw new Error("Cache tag cannot be empty after sanitization");
}
// Validate against common injection patterns
if (
sanitized.includes("<") ||
sanitized.includes(">") ||
sanitized.includes('"')
) {
throw new Error('Cache tag contains invalid characters (<, >, ")');
}
return sanitized;
// Only allow [A-Za-z0-9._-]
const allowedTag = tag.trim();
if (!/^[A-Za-z0-9._-]+$/.test(allowedTag)) {
throw new Error(
"Cache tag contains invalid characters. Allowed: A-Z, a-z, 0-9, dash (-), underscore (_), period (.)"
);
}
return allowedTag;

Copilot uses AI. Check for mistakes.
@ascorbic ascorbic force-pushed the add-cache-handlers branch from d078e3b to 119004b Compare August 25, 2025 09:41
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 25, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
cache-handlers 7d6c604 Commit Preview URL

Branch Preview URL
Sep 02 2025, 08:18 AM

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