Closed jdcoldsmith closed 1 week ago
I see from this comment that people seem to be switching to using the ip-address package instead.
It looks like the feature used - guessing the current public ipv4 address of the server - is not available in ip-address.
Some options I see:
Source code of ip.address: https://github.com/indutny/node-ip/blob/main/lib/ip.js#L383
I propose exporting a new function in core lib, which loops through addresses by nic, picking the first non loopack ipv4 address found.
An enhancement would be allowing this value to be passed in to more places as a configuration object, to better handle cases where there are mutliple addresses that meet this criteria (if that is desired).
Great points @dmihalcik! Another possibility could be the public-ip package. It seems to be very straightforward and accomplish the small need of this package.
In our codebase we use only one method from ip
given it's always a call to ip.address()
without arguments, it's important to check what the defaults are
I think the defaults will be name="private"
and family="ipv4"
, given the last implementation
// currently used functon
// node_modules/ip/lib/ip.js
//
// ### function address (name, family)
// #### @name {string|'public'|'private'} **Optional** Name or security
// of the network interface.
// #### @family {ipv4|ipv6} **Optional** IP family of the address (defaults
// to ipv4).
//
// Returns the address for the network interface on the current system with
// the specified `name`:
// * String: First `family` address of the interface.
// If not found see `undefined`.
// * 'public': the first public ip address of family.
// * 'private': the first private ip address of family.
// * undefined: First address with `ipv4` or loopback address `127.0.0.1`.
//
ip.address = function (name, family) {
var interfaces = os.networkInterfaces();
//
// Default to `ipv4`
//
family = _normalizeFamily(family);
//
// If a specific network interface has been named,
// return the address.
//
if (name && name !== 'private' && name !== 'public') {
var res = interfaces[name].filter((details) => {
var itemFamily = _normalizeFamily(details.family);
return itemFamily === family;
});
if (res.length === 0) {
return undefined;
}
return res[0].address;
}
var all = Object.keys(interfaces).map((nic) => {
//
// Note: name will only be `public` or `private`
// when this is called.
//
var addresses = interfaces[nic].filter((details) => {
details.family = _normalizeFamily(details.family);
if (details.family !== family || ip.isLoopback(details.address)) {
return false;
} if (!name) {
return true;
}
return name === 'public' ? ip.isPrivate(details.address)
: ip.isPublic(details.address);
});
return addresses.length ? addresses[0].address : undefined;
}).filter(Boolean);
return !all.length ? ip.loopback(family) : all[0];
};
so looks like we need internal-ip from the same author as public-ip
specifically internalIpV4()
import {internalIpV4} from 'internal-ip';
console.log(await internalIpV4());
//=> '10.0.0.79'
but it's just my guess based on quick analyses of the libraries
UPDATE
this is confusing though
return name === 'public' ? ip.isPrivate(details.address)
: ip.isPublic(details.address);
did a test locally
// index.mjs
import ip from "ip";
import { internalIpV4 } from "internal-ip";
import { publicIpv4 } from "public-ip";
console.log("ip.address()", ip.address());
console.log("await internalIpV4()", await internalIpV4());
console.log("await publicIpv4()", await publicIpv4());
> node index.mjs
ip.address() 192.168.1.104
await internalIpV4() 192.168.1.0
await publicIpv4() xxx.xxx.xxx.xxx (it's certainly not what we need)
Wait maybe I am missing something but ip.address()
and internalIpV4()
produced different results. Is that ok?
Wait maybe I am missing something but
ip.address()
andinternalIpV4()
produced different results. Is that ok?
no, it's not, updated the comment
given the difference, I think we actually need this https://github.com/silverwind/default-gateway
gonna test now
UPDATE
also not
// index.mjs
import ip from "ip";
import { gateway4sync } from "default-gateway";
import { internalIpV4 } from "internal-ip";
import { publicIpv4 } from "public-ip";
console.log("ip.address()", ip.address());
console.log("gateway4sync().gateway", gateway4sync().gateway);
console.log("await internalIpV4()", await internalIpV4());
console.log("await publicIpv4()", await publicIpv4());
> node index.mjs
ip.address() 192.168.1.104
gateway4sync().gateway 192.168.1.1
await internalIpV4() 192.168.1.0
await publicIpv4() xxx.xxx.xxx.xxx (it's certainly not what we need)
did a bit research on what's going on internally
internal-ip
should be the one we need, but it's either buggy, or I'm missing smth, but 0
at the end is not correct, it should just give 192.168.1.104
I can't find any working alternaitve though, feel free to suggest one
otherwise I can look into making our own utility for that, which is basically a simplified version of the old ip
package implementation
PS: e.g. storybook just implemented a custom utility in their codebase https://github.com/storybookjs/storybook/pull/27529/files
Any ETA on the fix? Thank you for all the good work here.
Found a working one, looks good https://www.npmjs.com/package/local-ip-url Will make a PR shortly
// index.mjs
import ip from "ip";
import localIpUrl from "local-ip-url";
import { gateway4sync } from "default-gateway";
import { internalIpV4 } from "internal-ip";
console.log("ip.address()", ip.address());
console.log("localIpUrl()", localIpUrl());
console.log("gateway4sync().gateway", gateway4sync().gateway);
console.log("await internalIpV4()", await internalIpV4());
> node index.mjs
ip.address() 192.168.1.104
localIpUrl() 192.168.1.104
gateway4sync().gateway 192.168.1.1
await internalIpV4() 192.168.1.0
Unfortunately local-ip-url
code seems to be a copy of ip
with same security issue, so it's not really fixing the problem, but rather hiding it and opening a possibility soon to run into the same security bug.
I figured internal-ip
worked well until v8 https://github.com/sindresorhus/internal-ip/issues/48
So if we use internal-ip@7.x.x
it's fine.
It does't have any checks for public IPs which I think is the root cause of the security issue, because such check if done wrongly can lead to opening certain attacks.
So we are fine to use internal-ip
without worrying about this security issue reappearing.
I'd like to thank you for the effort you've put in on this - I know you hit a bit of a pivot point but is there any ETA on the internal-ip@7.x.x
approach?
It's released. For exact versions of new packages you can check https://github.com/modernweb-dev/web/pull/2744
Thank you very much @bashmish for your efforts on this!
This issue is just meant to raise the flag about the current vulnerability with the
ip
package that@web/dev-server
depends on. It seems that there currently is no fix for this fromip
. https://github.com/advisories/GHSA-2p57-rm9w-gvfp