advanced_config is admin-only, but other fields are written into nginx
config as-is and the schema barely limited them. A non-admin could add
any directive through them:
- location path, forward_host and forward_path (any non-empty string)
- proxy host forward_host (a quote breaks out of set $server "...")
- redirection forward_domain_name and stream forwarding_host (their
pattern allowed ";", e.g. "example.com; return 200; #")
- access list IPv6 addresses ("%.+" zone id allowed "::1%x; allow all")
- domain names (a newline got out of the header comment)
Validation
- New lib/nginx-values.js holds the rules. Values can't contain
characters that end or change an nginx token: whitespace, control
characters, ; { } " ' # \, and no $ in a host. Location paths allow a
modifier and an unquoted or double-quoted URI, so =, ~, ~* and ^~
locations and quoted regexes keep working.
- The API schemas use the same patterns, including the certificate
schema's own copy of the domain rule. A test fails if they drift.
- generateConfig checks stored values again, so rows saved before these
rules can't inject either. The check is about nginx structure only:
a stored value the API would now reject, but which can't add
directives, still renders. If a value is unsafe, nothing is written
and the host is marked offline with the reason in nginx_err. The same
check covers per-location access lists and the Let's Encrypt request
config.
- htpasswd user names can't contain ":" or control characters, which
corrupted the file.
Fixes along the way
- IPv6 client addresses: drop zone ids, which nginx's allow/deny
rejects anyway, and fix broken escapes (literal "d" for digits, an
unescaped ".").
- Rewrite the domain rule used by redirection and stream hosts so it
can't backtrack exponentially. It accepts the same values.
- Pattern errors still say "must match pattern", but no longer quote
the regex.
Tests
- backend/test/nginx-values.test.js, run by `yarn test` and in
scripts/ci/test-and-build. Real request schemas reject an injection
set and accept valid values on every endpoint. Accepted values,
rendered through the real templates and parsed using nginx's
tokenizer rules, keep the same statement structure. The check
refuses unsafe stored rows.
- NginxValues.cy.js: injection attempts from a non-admin get 400, and
hosts using valid edge-case values come up with nginx_online true.
Security
- Check advanced_config permissions before creating a certificate on
host update (proxy, redirection and 404 hosts). A refused request
previously requested a Let's Encrypt certificate first, leaving an
orphan certificate and using up rate limit.
- Only trust X-NPM-Peer-Addr when it comes with a secret generated at
container start. The admin nginx sends both headers. Any proxy host or
stream forwarding to 127.0.0.1:3000 was also a loopback peer, so it
could forge the header and get a fresh rate-limit bucket on every
request. Without the secret (dev, or a mounted production.conf), the
header is ignored.
- Add a login limit per IP across all accounts (30 failed attempts a
minute) alongside the per-account one. Before this, one address could
spray passwords over any number of accounts. Behind a shared proxy,
everyone using that proxy shares this limit.
- Stop trusting X-Real-IP from loopback. It was only there for an older
version of the rate limiter, and it let a stream host forwarding to a
local proxy host forge the client IP and get past IP access lists.
Loopback now behaves as it does on master.
Correctness
- Refuse to delete a certificate that hosts still use (400, with counts
per host type). Their configs include its files, and a successful
revoke deleted them anyway, which broke nginx -t. Otherwise the
certificate is revoked and its files are removed even if the revoke
fails.
- Let non-admins clear a location's advanced config. Deleting the
location was already allowed and has the same effect, and a location
deleted and re-added at the same path arrives with an empty config,
which was refused.
- Clear proxy host grants (meta.proxy_host_ids) when visibility isn't
"user" or proxy hosts are hidden, and drop ids of deleted hosts.
Before, hidden grants came back when proxy hosts were shown again.
The backend enforces this; the permissions modal also stops sending
them.
- 404 host and redirection host modals now wait for the current user
to load, so an admin's advanced config change isn't dropped if they
submit before it loads.
Tests
- NonAdminHosts.cy.js: refused change doesn't create a certificate,
location rename and clear rules, grant clearing.
- Certificates.cy.js: deleting a certificate that is in use.
Security
- Redact certbot output before it reaches the API: strip the
certificate's DNS credential values, URL query strings and userinfo,
secret-looking key=value pairs and Bearer/Basic tokens. DNS plugins
can echo request URLs carrying API keys. The raw CommandError is no
longer attached as `previous`, since the error handler returns
`previous` to the client on certificate routes.
- Rate limit login/2FA on the admin nginx's real TCP peer, passed as
X-NPM-Peer-Addr ($realip_remote_addr), instead of X-Forwarded-For.
nginx trusts X-Real-IP from private ranges, so a client on the LAN or
Docker network could get a fresh bucket on every request. Behind
another proxy, limits now apply per account through that proxy.
- Bound rate limiter memory: hash keys over 128 chars and cap each store
at 50k entries, removing expired entries first and then the oldest.
Correctness
- Proxy host modal: send location advanced configs unchanged for
non-admins instead of looking them up by path, which dropped the
config from a renamed location.
- Location advanced config: locations that omit advanced_config now get
a config only after explicit ones are matched, so a new location at
an old path can't take the config from the renamed location.
- Certificate upload: check that the certificate matches the private
key (uploaded or stored) before writing, so a mismatched pair can't
break nginx -t.
- Certificate delete: keep the Let's Encrypt files when revoke fails and
hosts still use the certificate, so their configs still load.
Cleanup
- Make the rate-limit certbot hint first in the list, as its comment
says.
- Proxy host getCount uses the permissions access has already loaded
instead of querying user_permission again.
- Rate limiter: register the success refund listener in one place and
call it a fixed window, not a sliding one.
6003/6111/9103/9109/10000, plus 'Authentication error' and 'Invalid
access token') is checked before the rate-limit, DNS and HTTP hints, so
non-Cloudflare failures get a misleading Cloudflare message.
means upload() now reads a row with no private key, so an upload that
leaves out certificate_key wipes the stored key and writeCustomCert
writes undefined to privkey.pem.
Without a password to check, a login for an unknown account skipped bcrypt
and answered far sooner than one with a wrong password, which still told
which emails have an account.
- Add in-memory sliding window rate limiter for auth token endpoints with CI/test bypass and successful request exclusion
- Add user_permission meta column migration and Objection model configuration
- Support scoping allowed proxy hosts per user when in 'Created Items Only' visibility mode
- Restrict advanced nginx configuration editing on hosts to admin users only
- Fix user permissions route ID parsing and omit id from permission update payloads
- Add allowed proxy hosts selector in permissions modal and display permissions summary badges in users table
certbot deprecated --dns-route53-propagation-seconds in 2.5.0 and
removed it in 3.0.0, so requesting a Route 53 certificate with a
propagation value fails with "unrecognized arguments". The plugin
waits for the record through the AWS GetChange API instead.
Fixes#4702Fixes#5414
Co-authored-by: Pascaru <23747773+pascaruchan@users.noreply.github.com>
Some paths rendered host configs without fully loading the relations
the templates depend on, silently dropping access rules (and SSL):
- Enabling a proxy host loaded the access list without its clients or
auth items, so the host came back with no access restrictions.
- Requesting a Let's Encrypt certificate re-rendered every host sharing
its domains from bare rows, dropping access rules and SSL.
generateConfig now loads any referenced certificate or access list that
isn't fully loaded, so no caller can render a partial config. The two
callers above also fetch the relations themselves, and a warning is
logged when a host's access list is missing or has no rules.
Adds integration tests covering access lists across disable/enable.
Fixes#5919, #4286, #5710
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The private key of custom certificates was returned in meta by GET /api/nginx/certificates and GET /api/nginx/certificates/{id}. Add meta.certificate_key to omissions(), the same way meta.dns_provider_credentials is hidden. The key stays in the database and is still written to /data/custom_ssl on upload.
The stream table's forward_ip column was renamed to forwarding_host
back in migration 20210423103500_stream_domain.js, so the stream log
source labels were always showing "undefined" for the forwarding
target. Spotted by @jc21 in review.
Nginx Proxy Manager had no way to inspect application or nginx logs
from the web UI - admins had to shell into the container or read
`docker logs`. This adds an admin-only Logs page that can tail:
- the backend application log, now also mirrored to
/data/logs/backend.log (in addition to stdout) and rotated by the
logrotate timer that already runs every 2 days
- the Let's Encrypt/certbot log, which certbot already writes to
/data/logs/letsencrypt.log via its existing --logs-dir flag
- per-host nginx access/error logs (proxy, redirection, 404 and
stream hosts), with the file path always resolved server-side from
a validated host_type enum + numeric host_id, never from client
input
Reads use a reverse chunked scan (64KB chunks, capped at 5MB scanned
per request) instead of loading whole files into memory, and the
frontend polls every 5s only while the tab is focused and "Live" is
on, so this stays cheap on both CPU and memory. No new runtime
dependencies were added on either side.
Purely additive: two new admin-only endpoints
(GET /api/logs/sources, GET /api/logs/tail), no existing behaviour
changed.
Your CI caught this: the check passed on SQLite and never fired on the
stack where the database container runs on a different timezone from the
app, so a stale token stayed valid. The comparison was between a token's
`iat`, which is UTC seconds from Node, and `auth.modified_on`, which the
driver hands back interpreted in the app's timezone. With the app on
Australia/Brisbane and the database on UTC, that column comes back ten
hours in the past and the token always looks newer than the change.
Record the moment in `auth.meta.password_changed_at` instead, written by
`setPassword` with the same `Date.now()` clock that mints `iat`. Same
unit on both sides, one clock, and no timestamp parsing: the Date and
local-string branch is gone, and so is the whole-second flooring that
Postgres microseconds made necessary.
Rows written before this have no marker and revoke nothing until their
next password change, which is the safe direction to be wrong in.
The credentials file written for a DNS-01 challenge was only cleaned up when
certbot failed - the unlink sat in a catch block. On success the file stayed in
/etc/letsencrypt/credentials for the entire life of the certificate, holding a
live DNS provider API token in plaintext.
The file cannot simply be deleted at issuance, because certbot records its path
in the renewal config and reads it back on every `certbot renew`. So the renew
path now writes the file itself immediately before invoking certbot, and both
paths remove it in a finally block.
Net effect: the credentials exist on disk for the duration of a certbot run
rather than permanently. The value still lives in the certificates table, which
is unavoidable - it has to come from somewhere to be written at all.
renewLetsEncryptSslWithDnsChallenge reads the row directly from the model
because renew() sources its certificate from internalCertificate.get(), which
strips meta.dns_provider_credentials via omissions().
When `nginx -t` fails, configure() is meant to move the broken config to
<id>.conf.err so the failure can be inspected. renameConfigAsError()
unlinked the source file before renaming it, so the rename always failed
and the config was simply deleted. The deleteConfig() call after it then
removed any .err file left over from an earlier failure.
- unlink the destination .err file instead of the source
- return the rename promise so the delete does not race it
- pass delete_err_file = false so the new .err file survives
- drop the stale 4th argument in the success path, which silently made
delete_err_file false and left old .err files behind
- Use optional chaining for nullable checks
- Replace template literals with string literals for plain SQL
- Add node: protocol to fs/promises import in setup.js
A Stream Host with an IPv6 address as the Forward Host was accepted and
saved but never activated. The generated stream config rendered
`proxy_pass {{ forwarding_host }}:{{ forwarding_port }}` as e.g.
`fe80::528:3c87:e7bb:ab08:25`, which nginx rejects with
"invalid port in upstream" because an IPv6 literal must be wrapped in
square brackets before the port is appended (`[fe80::...]:25`).
Normalize the stream forward host in generateConfig (alongside the existing
per-host-type data massaging) using net.isIPv6(), so IPv6 hosts render as
`[address]:port` while IPv4 addresses and hostnames are emitted unchanged.
The mutation is applied to the deep-copied render object, so persisted and
audit data are unaffected.
Fixes#5740
Jamie Curnow
Alexis Loskoutoff
Abdulhafiz Davlatov
mikeyheneinandClaude Opus 5.5
Junker der ProvinzandPascaru
CnpPt
Amit Yadav
Dimas Rakha Wisnu
carlosalbertorg
José M. Requena Plens
Shawn Hank
vzagorovskiy
vgoer
Dimas R. Wisnu
addielarue