Skip to content

Commit eb12379

Browse files
mangyan1Zoey2936
andauthored
consolidation refactor with characterization tests (#9)
* small fix (extracted from NginxProxyManager#5835) * pin the dns-challenge certificate flow with characterization tests lock the current behavior of the certbot dns-01 path behind tests so a future change cannot silently remove dns challenge support again: the plugin catalog shape, pip install of the provider package, the exact certbot invocation (authenticator, credentials file, idn domains, propagation seconds), the 0600 credentials file, cleanup on failure, the revoke-then-renew renewal sequence, and fail-fast on unknown providers * pin the http auth, permission and crud contract with express-level tests * pin the crowdsec route contract before extracting its service layer * extract the crowdsec lapi client into internal/crowdsec.js and drop the redundant sendError * drop the redundant try/catch scaffolding express 5 already handles * type the api transport generically instead of blind any casts * split the crowdsec dashboard into per-tab components * pin the backend entrypoint to lf so local windows builds boot * document the internal refactor and its characterization coverage * apply biome formatting to the consolidation refactor * give the http characterization tests writable container-layout paths on the ci runner * diagnose the ci data dir permissions * chown the test data dirs inside the backend step since the runner resets the container per step * diagnose the chown behaviour inside the backend step * chown the test data dirs by uid since the runner user variable is empty * name the lapi machine key honestly and explain the sha256 cache fingerprint --------- Co-authored-by: Zoey <zoey@z0ey.de>
1 parent 5904167 commit eb12379

32 files changed

Lines changed: 2723 additions & 2486 deletions

.gitattributes

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
*.sh text eol=lf
33
*.patch text eol=lf
44
Dockerfile text eol=lf
5+
backend/index.js text eol=lf

.github/workflows/lint-and-format.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ jobs:
2626
version: 11.22.0
2727
- name: backend
2828
run: |
29+
# the http characterization tests boot the real app, which writes to
30+
# the same container-layout paths (/data, /usr/local/nginx) it uses
31+
# in production; the hosted runner only allows writes there as root
32+
# and every step gets a fresh container, so this must happen here
33+
sudo mkdir -p /data /usr/local/nginx/conf/conf.d
34+
sudo chown -R "$(id -u):$(id -g)" /data /usr/local/nginx
2935
cd backend
3036
pnpm install --frozen-lockfile
3137
pnpm biome ci --reporter=default --reporter=github

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ All notable changes to the NPMplus Security Fork are documented here. The fork u
4545
### Changed
4646

4747
- Restored upstream NPMplus runtime Certbot DNS-plugin installation so Cloudflare and other DNS challenges work out of the box; pinned pip and Certbot stay in the image and the pip packaging-tool scan findings are carried under a reviewed, expiring `.trivy/npmplus.yaml` baseline.
48+
- Refactored the internal code layout without behavior changes: the CrowdSec LAPI client moved to its own module, the redundant per-route error handling that Express 5 already performs was deleted, the frontend API transport is generically typed, and the CrowdSec dashboard was split into per-tab components. HTTP-level characterization tests now pin the auth, CRUD, CrowdSec route, and DNS-challenge certificate contracts before any future change can drift them, and the backend entrypoint is pinned to LF so local Windows image builds boot.
4849

4950
## v2.15.1-mangyan1.rc.4 - 2026-09-05
5051

backend/internal/crowdsec.js

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// The CrowdSec LAPI client: key-file and machine-token authentication, bounded
2+
// fetches, and the local config/log readers the dashboard routes build on.
3+
// Routes own presentation; this module owns every outbound CrowdSec call.
4+
import { createHash } from "node:crypto";
5+
import { readFile } from "node:fs/promises";
6+
import process from "node:process";
7+
import { fetchWithTimeout, readBoundedJson } from "../lib/bounded-fetch.js";
8+
import { debug, express as logger } from "../logger.js";
9+
import PACKAGE from "../package.json" with { type: "json" };
10+
11+
export const publicError = (message, status) => Object.assign(new Error(message), { public: true, status });
12+
13+
const LAPI_KEY_FILE = process.env.CROWDSEC_LAPI_KEY_FILE || "/data/crowdsec/lapi-ui.key";
14+
const LAPI_URL = process.env.CROWDSEC_LAPI_URL || "http://127.0.0.1:8080";
15+
const LAPI_MACHINE_ID = process.env.CROWDSEC_LAPI_MACHINE_ID || "npmplus-ui";
16+
const LAPI_MACHINE_KEY_FILE = process.env.CROWDSEC_LAPI_MACHINE_KEY_FILE || "/data/crowdsec/lapi-ui-machine.key";
17+
const CROWDSEC_BOUNCER_CONFIG_FILE = process.env.CROWDSEC_BOUNCER_CONFIG_FILE || "/data/crowdsec/crowdsec.conf";
18+
const MACHINE_TOKEN_TTL_MS = 30 * 1000;
19+
// crowdsec's lapi validates the client user agent against the registered
20+
// machine (a bare "node" agent is rejected as "bad user agent"), so every
21+
// request this backend makes identifies itself like the other bouncers do
22+
export const LAPI_USER_AGENT = `npmplus-ui-backend/${PACKAGE.version}`;
23+
const configuredTimeout = Number.parseInt(process.env.CROWDSEC_LAPI_TIMEOUT_MS || "5000", 10);
24+
const LAPI_TIMEOUT_MS = Number.isFinite(configuredTimeout) && configuredTimeout > 0 ? configuredTimeout : 5000;
25+
const LAPI_MAX_RESPONSE_BYTES = 2 * 1024 * 1024;
26+
const HONEYPOT_IP_PATTERN = /^[0-9a-fA-F.:]+$/;
27+
const HONEYPOT_LOG_PATH = process.env.ANUBIS_HONEYPOT_LOG_FILE || "/data/anubis/honeypot.addrs";
28+
const HONEYPOT_LOG_MAX_BYTES = 256 * 1024;
29+
30+
export const fetchCrowdsec = async (url, options = {}, timeoutMs = LAPI_TIMEOUT_MS) => {
31+
try {
32+
return await fetchWithTimeout(url, options, timeoutMs);
33+
} catch (err) {
34+
debug(logger, `CrowdSec request failed: ${err}`);
35+
throw publicError("crowdsec.unavailable", 502);
36+
}
37+
};
38+
39+
export const readCrowdsecJson = async (response, maxBytes = LAPI_MAX_RESPONSE_BYTES) => {
40+
try {
41+
return await readBoundedJson(response, maxBytes);
42+
} catch (err) {
43+
debug(logger, `CrowdSec response was invalid: ${err}`);
44+
throw publicError("crowdsec.invalid-response", 502);
45+
}
46+
};
47+
48+
// The dashboard needs to distinguish a quiet WAF from a disabled one. Read only
49+
// non-secret switches from the local bouncer config; never return its API key or
50+
// endpoint. A missing legacy file degrades to unknown instead of failing metrics.
51+
export const readAppsecConfiguration = async () => {
52+
try {
53+
const text = (await readFile(CROWDSEC_BOUNCER_CONFIG_FILE, "utf8")).slice(0, 64 * 1024);
54+
const setting = (name) => text.match(new RegExp(`^${name}=(.*)$`, "m"))?.[1]?.trim() ?? "";
55+
const url = setting("APPSEC_URL");
56+
const failureAction = setting("APPSEC_FAILURE_ACTION");
57+
const unreadableBody = setting("APPSEC_DROP_UNREADABLE_BODY");
58+
return {
59+
appsec_configured: Boolean(url),
60+
appsec_failure_action: ["deny", "passthrough"].includes(failureAction) ? failureAction : null,
61+
appsec_drop_unreadable_body: ["true", "false"].includes(unreadableBody) ? unreadableBody === "true" : null,
62+
};
63+
} catch (err) {
64+
debug(logger, `CrowdSec bouncer config unavailable for AppSec status: ${err}`);
65+
return {
66+
appsec_configured: null,
67+
appsec_failure_action: null,
68+
appsec_drop_unreadable_body: null,
69+
};
70+
}
71+
};
72+
73+
// the honeypot log lives on the anubis data volume, readable by the backend;
74+
// newest-last per the writer (anubis appends), capped at the configured size
75+
export const readRecentHoneypotIps = async () => {
76+
let content;
77+
try {
78+
content = await readFile(HONEYPOT_LOG_PATH, "utf8");
79+
} catch (err) {
80+
if (err?.code === "ENOENT") return { status: "waiting", items: [] };
81+
debug(logger, `Anubis honeypot log is unreadable: ${err}`);
82+
return { status: "unavailable", items: [] };
83+
}
84+
if (content.length > HONEYPOT_LOG_MAX_BYTES) {
85+
content = content.slice(-HONEYPOT_LOG_MAX_BYTES);
86+
}
87+
const items = content
88+
.split("\n")
89+
.map((line) => line.trim())
90+
.filter((line) => line.length > 0 && line.length <= 45)
91+
.filter((line) => HONEYPOT_IP_PATTERN.test(line));
92+
return { status: "ready", items };
93+
};
94+
95+
export const lapiFetch = async (path) => {
96+
let key = "";
97+
try {
98+
key = (await readFile(LAPI_KEY_FILE, "utf8")).trim();
99+
} catch {
100+
// Handled below with a stable, localizable error code.
101+
}
102+
if (!key) throw publicError("crowdsec.not-wired", 503);
103+
104+
const response = await fetchCrowdsec(`${LAPI_URL}${path}`, { headers: { "X-Api-Key": key } });
105+
if (!response.ok) {
106+
throw publicError(
107+
response.status === 401 || response.status === 403 ? "crowdsec.bad-key" : "crowdsec.lapi-error",
108+
502,
109+
);
110+
}
111+
return readCrowdsecJson(response);
112+
};
113+
114+
let machineTokenCache = null;
115+
116+
const lapiLogin = async () => {
117+
let machineKey = "";
118+
try {
119+
machineKey = (await readFile(LAPI_MACHINE_KEY_FILE, "utf8")).trim();
120+
} catch {
121+
// Handled below with a stable, localizable error code.
122+
}
123+
if (!machineKey) throw publicError("crowdsec.not-wired-machine", 503);
124+
125+
// sha256 here is only an in-memory cache fingerprint to notice key-file
126+
// changes between requests; the key itself is never stored or compared
127+
// against persisted verifiers.
128+
const fingerprint = createHash("sha256").update(machineKey).digest("hex");
129+
if (
130+
machineTokenCache?.fingerprint === fingerprint &&
131+
machineTokenCache.expiresAt > Date.now() &&
132+
machineTokenCache.token
133+
) {
134+
return machineTokenCache.token;
135+
}
136+
137+
const response = await fetchCrowdsec(`${LAPI_URL}/v1/watchers/login`, {
138+
method: "POST",
139+
headers: { "Content-Type": "application/json", "User-Agent": LAPI_USER_AGENT },
140+
body: JSON.stringify({ machine_id: LAPI_MACHINE_ID, password: machineKey }),
141+
});
142+
if (!response.ok) {
143+
throw publicError(
144+
response.status === 401 || response.status === 403 ? "crowdsec.bad-machine-key" : "crowdsec.lapi-error",
145+
502,
146+
);
147+
}
148+
const body = await readCrowdsecJson(response, 64 * 1024);
149+
if (typeof body?.token !== "string" || !body.token) throw publicError("crowdsec.invalid-response", 502);
150+
151+
machineTokenCache = {
152+
fingerprint,
153+
token: body.token,
154+
expiresAt: Date.now() + MACHINE_TOKEN_TTL_MS,
155+
};
156+
return body.token;
157+
};
158+
159+
export const lapiMachineFetch = async (
160+
path,
161+
method = "GET",
162+
mayRetry = true,
163+
{ body = null, readJson = readCrowdsecJson } = {},
164+
) => {
165+
const token = await lapiLogin();
166+
const options = {
167+
method,
168+
headers: { Authorization: `Bearer ${token}`, "User-Agent": LAPI_USER_AGENT },
169+
};
170+
if (body !== null) {
171+
options.headers["Content-Type"] = "application/json";
172+
options.body = JSON.stringify(body);
173+
}
174+
const response = await fetchCrowdsec(`${LAPI_URL}${path}`, options);
175+
if (response.status === 401 || response.status === 403) {
176+
machineTokenCache = null;
177+
if (mayRetry) return lapiMachineFetch(path, method, false, { body, readJson });
178+
throw publicError("crowdsec.bad-machine-key", 502);
179+
}
180+
if (!response.ok) throw publicError("crowdsec.lapi-error", 502);
181+
return readJson(response);
182+
};

backend/internal/user.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,15 +514,16 @@ const internalUser = {
514514

515515
let permissions;
516516

517+
const { id, ...permissionData } = data;
517518
const existing_auth = await userPermissionModel.query().where("user_id", user.id).first();
518519

519520
if (existing_auth) {
520521
permissions = await userPermissionModel
521522
.query()
522523
.where("user_id", user.id)
523-
.patchAndFetchById(existing_auth.id, { user_id: user.id, ...data });
524+
.patchAndFetchById(existing_auth.id, { user_id: user.id, ...permissionData });
524525
} else {
525-
permissions = await userPermissionModel.query().insertAndFetch({ user_id: user.id, ...data });
526+
permissions = await userPermissionModel.query().insertAndFetch({ user_id: user.id, ...permissionData });
526527
}
527528

528529
await internalAuditLog.add(access, {

backend/routes/audit-log.js

Lines changed: 38 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -26,30 +26,25 @@ router
2626
* Retrieve all logs
2727
*/
2828
.get(async (req, res, next) => {
29-
try {
30-
const data = await validator(
31-
{
32-
additionalProperties: false,
33-
properties: {
34-
expand: {
35-
$ref: "common#/properties/expand",
36-
},
37-
query: {
38-
$ref: "common#/properties/query",
39-
},
29+
const data = await validator(
30+
{
31+
additionalProperties: false,
32+
properties: {
33+
expand: {
34+
$ref: "common#/properties/expand",
35+
},
36+
query: {
37+
$ref: "common#/properties/query",
4038
},
4139
},
42-
{
43-
expand: typeof req.query.expand === "string" ? req.query.expand.split(",") : null,
44-
query: typeof req.query.query === "string" ? req.query.query : null,
45-
},
46-
);
47-
const rows = await internalAuditLog.getAll(res.locals.access, data.expand, data.query);
48-
res.status(200).send(rows);
49-
} catch (err) {
50-
debug(logger, `${req.method.toUpperCase()} ${req.originalUrl}: ${err}`);
51-
next(err);
52-
}
40+
},
41+
{
42+
expand: typeof req.query.expand === "string" ? req.query.expand.split(",") : null,
43+
query: typeof req.query.query === "string" ? req.query.query : null,
44+
},
45+
);
46+
const rows = await internalAuditLog.getAll(res.locals.access, data.expand, data.query);
47+
res.status(200).send(rows);
5348
});
5449

5550
/**
@@ -70,35 +65,30 @@ router
7065
* Retrieve a specific entry
7166
*/
7267
.get(async (req, res, next) => {
73-
try {
74-
const data = await validator(
75-
{
76-
required: ["event_id"],
77-
additionalProperties: false,
78-
properties: {
79-
event_id: {
80-
$ref: "common#/properties/id",
81-
},
82-
expand: {
83-
$ref: "common#/properties/expand",
84-
},
68+
const data = await validator(
69+
{
70+
required: ["event_id"],
71+
additionalProperties: false,
72+
properties: {
73+
event_id: {
74+
$ref: "common#/properties/id",
75+
},
76+
expand: {
77+
$ref: "common#/properties/expand",
8578
},
8679
},
87-
{
88-
event_id: req.params.event_id,
89-
expand: typeof req.query.expand === "string" ? req.query.expand.split(",") : null,
90-
},
91-
);
80+
},
81+
{
82+
event_id: req.params.event_id,
83+
expand: typeof req.query.expand === "string" ? req.query.expand.split(",") : null,
84+
},
85+
);
9286

93-
const item = await internalAuditLog.get(res.locals.access, {
94-
id: data.event_id,
95-
expand: data.expand,
96-
});
97-
res.status(200).send(item);
98-
} catch (err) {
99-
debug(logger, `${req.method.toUpperCase()} ${req.originalUrl}: ${err}`);
100-
next(err);
101-
}
87+
const item = await internalAuditLog.get(res.locals.access, {
88+
id: data.event_id,
89+
expand: data.expand,
90+
});
91+
res.status(200).send(item);
10292
});
10393

10494
export default router;

0 commit comments

Comments
 (0)