Service last-access accounting - #451
Open
runleveldev wants to merge 17 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end “service last-access” accounting so the platform can surface which proxied services (and containers) are actively used, with throttled reporting from the agent proxy and server-stamped persistence in the manager, plus UI display support.
Changes:
- Agent: inject njs-based accounting hooks into rendered nginx config (HTTP mirror + stream access), including a localhost relay for stream TLS limitations.
- Manager: add
Services.lastAccessedAt+ migration, exposePOST /api/v1/services/:id/last-access, and include per-service + container rollup serialization + OpenAPI updates. - Client: add shared
formatRelativeTimeand display/sort a “Last Access” column; refactor Agents list to use the shared formatter.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| create-a-container/utils/agent-config.js | Adds service.id into the agent config snapshot for accounting reports. |
| create-a-container/utils/tests/agent-config.test.js | Tests snapshot includes service ids and ETag determinism. |
| create-a-container/routers/api/v1/index.js | Mounts the new /services API router. |
| create-a-container/routers/api/v1/containers.js | Serializes lastAccessedAt per-service and a container-level rollup. |
| create-a-container/routers/api/v1/tests/containers.serialize.test.js | Unit tests for the container/service lastAccessedAt serialization and rollup. |
| create-a-container/resources/services/validator.js | Zod validator for :id path param. |
| create-a-container/resources/services/service.js | Service-layer logic for recording access + 404 on unknown id. |
| create-a-container/resources/services/router.js | Adds POST /:id/last-access endpoint with localhost/admin auth model. |
| create-a-container/resources/services/repository.js | Implements a single-statement UPDATE to stamp lastAccessedAt. |
| create-a-container/resources/services/controller.js | Controller wiring for the record-access endpoint (204). |
| create-a-container/resources/services/tests/services.api.test.js | API tests for localhost/admin/non-admin/unknown-id/invalid-id cases. |
| create-a-container/openapi.v1.yaml | Documents new endpoint and adds lastAccessedAt fields to schemas. |
| create-a-container/models/service.js | Adds lastAccessedAt column to the Sequelize model. |
| create-a-container/migrations/20260811000000-add-service-last-accessed-at.js | Migration adding/removing Services.lastAccessedAt. |
| create-a-container/client/src/pages/agents/AgentsListPage.tsx | Refactors check-in formatting to shared formatRelativeTime. |
| create-a-container/client/src/lib/types.ts | Adds lastAccessedAt to container/service client types. |
| create-a-container/client/src/lib/formatRelativeTime.ts | New shared relative-time formatting utility. |
| create-a-container/client/src/components/containers/ContainersDataGrid.tsx | Adds “Last Access” column rendering/sorting using formatRelativeTime. |
| agent/test/nginx-template.test.js | Adds render tests pinning nginx template accounting hooks. |
| agent/templates/nginx.conf.ejs | Injects accounting module/dicts/vars, mirror subrequest, stream hooks, and relay server. |
| agent/src/types.ts | Adds id to HttpService/StreamService types consumed by the agent. |
| agent/src/index.ts | Passes agent config into apply to support secret-bearing templates. |
| agent/src/apply.ts | Threads AgentConfig into render, renders nginx with accounting vars, and writes nginx.conf as 0600. |
| agent/package.json | Adds a node test script. |
| agent/njs/accounting.js | New njs module implementing HTTP/stream accounting + relay forwarding. |
| agent/Makefile | Enables real test target and installs njs/ into the staged agent tree. |
| agent/.fpm | Adds nginx js module and CA cert dependencies needed by accounting fetch. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… to http{}
Move the stream-report relay off TCP 127.0.0.1:1985 to a unix socket
(/run/nginx-osaas-relay.sock), keeping it off the network entirely. The
stream fetch uses the njs unix-socket URL form (trailing ':' before the
URI) and sends an explicit Host header, which njs otherwise derives from
the socket path and nginx rejects with 400.
Hoist resolver, js_fetch_timeout, and js_fetch_trusted_certificate to the
http{} block so the mirror location and the relay server inherit them
instead of each carrying a copy.
Verified on debian:trixie (nginx 1.26.3 / njs 0.8.9): nginx -t passes on
the 0600 full render, and a stream connection delivers exactly one
correctly-shaped POST through the socket relay to the manager.
runleveldev
force-pushed
the
accounting-pr
branch
from
August 12, 2026 15:11
c47c3e8 to
fe1dae3
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…eserved name)
The notifications migration (from the notification-queue feature) creates a
"ctid" column, which Postgres reserves as a system column on every table.
CREATE TABLE therefore fails with 42701 ("column name \"ctid\" conflicts with
a system column name"), and since migrations run at startup the manager cannot
boot on Postgres at all. It only slipped through because the test suite runs on
SQLite, where "ctid" is not reserved.
Rename the physical column to "containerId" and map the model's `ctid`
attribute onto it via Sequelize's `field:` option, so the API/JSON/query
surface (webhook payload, serializer, repository lookups, validator, tests)
is completely unchanged — only the column name differs.
The migration has never applied successfully on Postgres (it fails on CREATE
TABLE, so no table and no SequelizeMeta row exist), so editing the migration
in place is safe — no follow-up migration is needed.
Verified against real Postgres 16: CREATE TABLE now succeeds with a
containerId column, and Notification.create({ ctid }) / findOne({ where:
{ ctid } }) round-trip correctly onto that column.
…warn level
Two fixes to the nginx last-access accounting on the manager's own embedded
agent:
1. dnsmasq did not resolve "localhost". The rendered dnsmasq config uses
`no-hosts`, so /etc/hosts (where 127.0.0.1 localhost lives) is ignored, and
there was no other source for the name. nginx's `resolver 127.0.0.1` (this
dnsmasq) therefore failed the njs accounting module's ngx.fetch to the
manager's own http://localhost:3000, logged as:
js: osaas accounting: "localhost" could not be resolved (3: Host not found)
Add `address=/localhost/127.0.0.1` and `.../::1` so localhost (and
*.localhost) resolve to loopback, per RFC 6761. Verified against real
dnsmasq on trixie: localhost A -> 127.0.0.1, AAAA -> ::1, foo.localhost ->
127.0.0.1, with no wildcarding of other names.
2. The accounting module logged via r.log/s.log, which write at njs's `info`
level. The rendered error_log threshold is `notice`, one level above info,
so every fail-open diagnostic was silently dropped. These lines only fire
on actual failures (non-204 responses, fetch exceptions), so raise them to
r.warn/s.warn (warning level) — visible under the `notice` threshold
without turning on info-level noise.
Adds a dnsmasq conf.ejs render test pinning the localhost override.
… accounting The service-accounting router (POST /api/v1/services/:id/last-access) had a verbatim copy of the agent check-in's auth logic (localhost bypass, else apiAuth + apiAdmin). Extract it once as localhostOrAdmin in middlewares/api (next to apiAuth/apiAdmin, same lazy isLocalhostRequest require the csrfGuard already uses) and have both routers use it. Behavior is unchanged; the duplicated checkinAuth/accountingAuth functions are removed.
isLocalhostRequest was defined in middlewares/index but its only consumers
are in middlewares/api (csrfGuard and localhostOrAdmin), which imported it
twice via lazy require('./index') to sidestep a load-order cycle. Move the
function to api.js next to its callers: both lazy requires and the cycle
guard disappear, and the duplicate destructure is gone. Removed from the
index barrel's exports (nothing imports it from there); api.js exports it
for discoverability alongside the other guards.
Add session-authenticated coverage for POST /api/v1/services/:id/last-access: a remote (X-Forwarded-For) session-cookie request is 403 without an X-CSRF-Token and 204 with a valid one. Guards against the Bearer/localhost CSRF exemptions silently regressing into a hole for cookie-authenticated callers. Verified against real Postgres 16 (8/8 in the suite). Addresses the reviewer note on services.api.test.js.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Records a per-service last-access timestamp so the platform can tell which proxied services are actually being used (e.g. to spot idle containers). Collection happens on the agent's nginx proxy with zero impact on request processing, and the manager is updated at most once per service per 10 minutes.
How it works
Agent (nginx proxy) — a static njs module (
agent/njs/accounting.js) wired intonginx.conf.ejs:mirrorsubrequest (js_content accounting.http_record), so it runs alongsideproxy_passand never adds latency to the client response.mirror_request_body offkeeps uploads unbuffered (Why uploads crawled behind our proxy — and the 15× fix #395). The mirror runs after the auth gate, so auth-rejected/scanner traffic doesn't count.js_access accounting.stream_record(fire-and-forget, connection setup never waits).timeout=10m+ atomicdict.add()is the entire throttle: exactly one report per service per 10-minute window, race-free across workers, and preserved across nginx reloads (shm inheritance).Manager (create-a-container):
Services.lastAccessedAtcolumn + migration.POST /api/v1/services/:id/last-access— auth mirrors the agent check-in precedent (localhost bypass OR admin Bearer), a singleUPDATEstatement, timestamp stamped server-side (agent clocks never trusted). 204 / 400 / 401 / 403 / 404.lastAccessedAtplus a container-level rollup (max across services). OpenAPI + client types updated.Client: shared
formatRelativeTimeutil + a sortable "Last Access" column on the containers dashboard;AgentsListPagerefactored onto the shared formatter.Notable design decisions (validated against Debian trixie)
js_engine qjsnorstate=dict persistence (both need 0.9.1). The module runs on the default njs engine (namedaccounting.js), and there's nostate=: reloads preserve the throttle, but a full nginx restart resets the windows — one extra benign report per active service. Accepted.NGX_STREAM_SSL, so streamngx.fetch()can't do TLS. Stream servers therefore report to a localhost-only http relay listening on a unix socket (/run/nginx-osaas-relay.sock,js_content accounting.relay) that forwards to the manager over TLS with the Bearer credential. The relay validates the path against^/api/v1/services/(\d+)/last-access$— the embedded credential can only ever produce a last-access stamp. (resolver/js_fetch_timeout/js_fetch_trusted_certificateare declared once athttp {}level and inherited.)nginx.confis now written0600(it embeds the admin API key); packaging addslibnginx-mod-http-js,libnginx-mod-stream-js, andca-certificates.Incidental fix (not part of the feature)
fix(create-a-container): rename Notifications ctid column— the notification-queue migration (frommain) creates actidcolumn, which Postgres reserves as a system column on every table, soCREATE TABLEfails with42701and the manager cannot boot on Postgres at all. (It slipped through because tests run on SQLite, wherectidis not reserved.) The physical column is renamed tocontainerIdand the model maps itsctidattribute onto it via Sequelize'sfield:, so the API/JSON/query surface is unchanged. Surfaced here because the rebase onto currentmainpulled this migration into the branch and it blocks startup; verified against real Postgres 16 (create +ctidattribute round-trip).Testing
node --test) — render tests pinning the accounting hooks (presence in service blocks, absence in default/landing/wildcard/503 blocks, stream relay unix socket, single API-key occurrence, http-level fetch-directive hoisting).nginx -tin a debian:trixie container (all branches), plus a live two-hop stream→socket-relay→manager delivery test.Notification.create({ ctid })/findOne({ where: { ctid } })round-trip onto thecontainerIdcolumn).Notes for reviewers
js/missing-rate-limitingon the new/services/:id/last-accessroute was dismissed as accepted debt: it's the same trust model as the existing agent check-in (agents.js:30, which carries the identical open alert), the repo has no rate-limiting middleware today, and the endpoint is machine-facing and throttled to ≤1 req/service/10min upstream in nginx.