Skip to content

feat: add IP filter middleware#3035

Merged
bartlomieju merged 46 commits into
freshframework:mainfrom
Octo8080X:feature/add_ip_restriction_middleware
Mar 29, 2026
Merged

feat: add IP filter middleware#3035
bartlomieju merged 46 commits into
freshframework:mainfrom
Octo8080X:feature/add_ip_restriction_middleware

Conversation

@Octo8080X
Copy link
Copy Markdown
Contributor

@Octo8080X Octo8080X commented Jun 14, 2025

Add an IP restriction middleware for Fresh, ported from Hono's IP restriction middleware.

Supports deny lists, allow lists, and CIDR subnet matching. Deny rules take precedence over allow rules.

app.use(ipFilter({
  denyList: ["192.168.1.10", "10.0.0.0/8"],
  allowList: ["192.168.1.0/24"],
}));

Closes #3011

Copy link
Copy Markdown
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

First pass

Comment thread src/middlewares/ip_restriction.ts Outdated
Comment on lines +4 to +24
export type AddressType = "IPv4" | "IPv6";

export type NetAddrInfo = {
/**
* Transport protocol type
*/
transport?: "tcp" | "udp";
/**
* Transport port number
*/
port: number;
address: string;
addressType?: AddressType;
};

export interface ConnInfo {
/**
* Remote information
*/
remote: NetAddrInfo;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These types already exist within the Deno runtime API. See https://docs.deno.com/api/deno/network.

Comment thread src/middlewares/ip_restriction_test.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Each test should be wrapped in a Deno.test() call.

Copy link
Copy Markdown
Contributor Author

@Octo8080X Octo8080X Jun 14, 2025

Choose a reason for hiding this comment

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

@iuioiua san.

Thanks for your comment!
Sorry if I’m mistaken, but I thought when writing in a BDD style, Deno.test would be replaced by describe. Was I incorrect about that?

https://jsr.io/@std/testing/doc/bdd#:~:text=()%2C%2018)%3B%0A%7D)%3B-,Mixed%20test%20grouping,-Both%20nested%20test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I looked at the other code as well.
I understand now that using Deno.test instead of a BDD style is the standard.
I’ll prepare a fix.

Comment thread src/middlewares/ip_restriction.ts Outdated
Comment thread src/middlewares/ip_restriction.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a lot of IP address logic here that might be valuable in other domains. I recommend creating an issue for the Standard Library for any useful utilities to @std/net.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The part you’re pointing out is indeed a generic utility — in Hono, it’s placed in utils as well.
I’ll go ahead and create an issue for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@iuioiua

I have created an issue.Thanks.
denoland/std#6722

@Octo8080X
Copy link
Copy Markdown
Contributor Author

@iuioiua
Hello.
The processing logic for IP addresses has been refactored and isolated into @std/net.
The middleware has been subsequently reconstructed utilizing this new module. Your review is requested.

import type { Middleware } from "./mod.ts";
import { isIPv4, isIPv6, matchSubnets } from "@std/net/unstable-ip";

export type AddressType = "IPv6" | "IPv4" | "none";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest using Deno.NetworkInterfaceInfo.family instead of this new type.

Comment on lines +26 to +33
function findType(addr: string): AddressType {
if (isIPv4(addr)) {
return "IPv4";
} else if (isIPv6(addr)) {
return "IPv6";
}
return "none";
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See first comment.

Suggested change
function findType(addr: string): AddressType {
if (isIPv4(addr)) {
return "IPv4";
} else if (isIPv6(addr)) {
return "IPv6";
}
return "none";
}
function findType(addr: string): Deno.NetworkInterfaceInfo.family | undefined {
if (isIPv4(addr)) {
return "IPv4";
} else if (isIPv6(addr)) {
return "IPv6";
}
return undefined;
}

Comment on lines +20 to +22
headers: {
"Content-Type": "text/plain",
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not needed. The content-type header by default is "text/plain;charset=UTF-8" anyway.

Suggested change
headers: {
"Content-Type": "text/plain",
},

Comment on lines +6 to +9
export interface IPRestrictionRules {
denyList?: string[];
allowList?: string[];
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This interface and its properties should have JSDocs.

Comment on lines +82 to +84
if (!ctx.info.remoteAddr.hostname) {
return blockError();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC, this conditional will never be true because ctx.info.remoteAddr.hostname is always defined.

Suggested change
if (!ctx.info.remoteAddr.hostname) {
return blockError();
}

* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}))
* ```
*
* @example custom error handling
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit

Suggested change
* @example custom error handling
* @example Custom error handling

* @example custom error handling
* ```ts
* const customOnError: IpRestrictionOptions = {
* onError: () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should show how the onErrors parameters can be used here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've gone ahead and added the usage examples.

Comment on lines +11 to +15
function buildMatcher(ipList: string[]) {
return (addr: string) => {
return matchSubnets(addr, ipList);
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not needed if we go ahead with using matchSubnets inline, as stated below.

Suggested change
function buildMatcher(ipList: string[]) {
return (addr: string) => {
return matchSubnets(addr, ipList);
};
}


const denyMatcher = buildMatcher(denyList);
const allowMatcher = buildMatcher(allowList);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something like this will avoid the need for if (options?.onError) conditionals and being inline makes the behaviour of onError more immediately understood.

Suggested change
const onError = options?.onError ?? (() =>
new Response("Forbidden", {
status: 403,
headers: {
"Content-Type": "text/plain",
},
}));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've made the adjustment to make it inline. Thanks!

* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}, customOnError))
* ```
*/
export function ipRestriction<State>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not strongly for or against this name... Are we sure about this name? Any other alternatives?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@iuioiua
How about 'IpRule' or 'IpFilter' instead of 'IP Restriction'? They sound pretty good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ipFilter sounds okay to me.

Copy link
Copy Markdown
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Getting closer to being ready, IMO. Few nits. HMU on Discord (same username) if you want to chat about any of this.

* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}, customOnError))
* ```
*/
export function ipRestriction<State>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ipFilter sounds okay to me.

}

if (matchSubnets(addr, rules.allowList || [])) {
const res = await ctx.next();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const res = await ctx.next();
return ctx.next();

Again, awaiting ctx.next() adds no value.

}

if ((rules.allowList || []).length === 0) {
return await ctx.next();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return await ctx.next();
return ctx.next();

Ditto

return await ctx.next();
} else {
if (options?.onError) {
return await options.onError({ addr, type }, ctx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return await options.onError({ addr, type }, ctx);
return options.onError({ addr, type }, ctx);

Ditto

Comment on lines +83 to +84
): Middleware<State> {
return async function ipRestriction<State>(ctx: Context<State>) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
): Middleware<State> {
return async function ipRestriction<State>(ctx: Context<State>) {
): Middleware<State> {
const onBlock = options.onError ?? () => new Response("Forbidden", { status: 403 });
return async function ipRestriction<State>(ctx: Context<State>) {

Reminder: avoids having to write if (options?.onError) blocks and makes things clear more immediately.

Comment on lines +102 to +105
if (options?.onError) {
return options.onError({ addr, type }, ctx);
}
return blockError();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto. See comment about onError being defined within the exported function.

Comment on lines +116 to +118
if (options?.onError) {
return await options.onError({ addr, type }, ctx);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto. See comment about onError being defined within the exported function.

/**
* IP restriction Middleware for Fresh.
*
* @param rules rules `{ denyList: string[], allowList: string[] }`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param rules rules `{ denyList: string[], allowList: string[] }`.
* @param rules Deny and allow rules object.

How you had it isn't how @param JSDoc tags should be written. They need to be written in natural human language describing what the variable is.

* ```ts
* const app = new App<State>()
*
* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}))
* app.use(ipRestriction({
* denyList: ["192.168.1.10", "2001:db8::1"],
* }));

Nit

* return new Response("custom onError", { status: 401 });
* },
* };
* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}, customOnError))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* app.use(ipRestriction({denyList: ["192.168.1.10", "2001:db8::1"]}, customOnError))
* app.use(ipRestriction({
* denyList: ["192.168.1.10", "2001:db8::1"]
* }, customOnError));

Nit

Copy link
Copy Markdown
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Getting closer.

Comment thread deno.lock
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these changes be reverted?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think ip_filter is necessary because it relies on @std/net.

Comment on lines +42 to +50
export interface ipFilterOptions {
onError?: <State>(
remote: {
addr: string;
type: Deno.NetworkInterfaceInfo["family"] | undefined;
},
ctx: Context<State>,
) => Response | Promise<Response>;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This interface and its properties need documentation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have addressed this matter.

}

const addr = ctx.info.remoteAddr.hostname;
const type = findType(addr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think whether you're using addr and type is unrelated to my point 😛

See my suggestion below.

}

const addr = ctx.info.remoteAddr.hostname;
const type = findType(addr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const type = findType(addr);
const type = IPv4(addr) ? "IPv4" : "IPv6";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I seem to have been overthinking it unnecessarily. I would like to adopt your proposal.

Comment on lines +97 to +99
if (type == undefined) {
return blockError();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I do think it's unnecessary because type can only ever be IPv4 or IPv6.

Comment on lines +95 to +97
throw new TypeError(
"Unsupported transport protocol. TCP & UDP is supported.",
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new TypeError(
"Unsupported transport protocol. TCP & UDP is supported.",
);
return ctx.next();

Comment on lines +85 to +92
if (
ctx.info.remoteAddr.transport !== "udp" &&
ctx.info.remoteAddr.transport !== "tcp"
) {
throw new TypeError(
"Unsupported transport protocol. TCP & UDP is supported.",
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, to correct my stance, yes, we should return early if not UDP or TCP traffic, because an IP address isn't being used. But I don't think throwing is the correct behaviour. Instead, we should just pass through. See my suggestion above.

rules: IpFilterRules,
options?: ipFilterOptions,
): Middleware<State> {
const onBlock = options?.onError ?? (() => blockError());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const onBlock = options?.onError ?? (() => blockError());
const onBlock = options?.onError ?? (() => new Response("Forbidden", { status: 403 }));

So the reader doesn't have to scroll back up to see what onBlock does by default (since it's simple anyway).

Comment on lines +115 to +118
if ((rules.allowList || []).length === 0) {
return ctx.next();
} else {
return onBlock({ addr, type }, ctx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ((rules.allowList || []).length === 0) {
return ctx.next();
} else {
return onBlock({ addr, type }, ctx);
return onBlock({ addr, type }, ctx);

I believe this first if statement is already covered by the preceding allow block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this section is needs.

This is because there were cases where the permission list was not explicitly set.
However, as pointed out, some parts seemed unnecessary, so I have reorganized the content.

Thank you

}

/**
* IP restriction Middleware for Fresh.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should explain this filter. I.e. the precedence of rules. So deny rules take precedence over allow rules and traffic not matching any rule is denied by default (implicit deny).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I mentioned this matter.

Thankyou

@Octo8080X Octo8080X requested a review from iuioiua November 22, 2025 14:02
Copy link
Copy Markdown
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏾

@Octo8080X
Copy link
Copy Markdown
Contributor Author

@marvinhagemeister

Thank you for the review on this PR a while back. It’s been about two months since iuioiua-san kindly provided feedback, and I truly appreciate the support. Please let me know if there is anything additional I can do to help get this merged—I'm ready to jump back in and make any necessary updates immediately.

- Rename ipFilterOptions to IpFilterOptions (PascalCase convention)
- Rename onError to onBlocked (clearer naming)
- Remove unused undefined from type field
- Fix typos: remortAddr -> remoteAddr, arrow -> allow
- Remove Japanese comments
- Export IpFilterOptions from mod.ts
- Simplify test helper (remove unnecessary branching)
- Update JSDoc examples

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bartlomieju bartlomieju changed the title Feature/add ip restriction middleware feat: add IP filter middleware Mar 26, 2026
bartlomieju and others added 2 commits March 27, 2026 07:49
Add docs/latest/plugins/ip-filter.md covering deny lists, allow lists,
CIDR subnet matching, combined rules, and custom blocked responses.
Register the page in the TOC under Plugins.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bartlomieju bartlomieju merged commit 566a6ce into freshframework:main Mar 29, 2026
7 checks passed
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.

suggestion: Expansion of official middleware for Fresh

4 participants