Skip to content

fix: serve the console metrics dashboard to admin sessions only - #727

Merged
LeeroyHannigan merged 7 commits into
mainfrom
fix/console-metrics-admin-only
Nov 5, 2026
Merged

LeeroyHannigan merged 7 commits into
mainfrom
fix/console-metrics-admin-only

Conversation

@LeeroyHannigan

@LeeroyHannigan LeeroyHannigan commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

What

The console metrics dashboard and its data feed now serve admin sessions only. The Settings and Cache pages were already admin only. Now the three pages refuse an IAM-user session in the same way.

  • GET /console/metrics, GET /console/settings, GET /console/cache and POST /console/cache/invalidate redirect an IAM-user session to /console. They share one check, require_admin. Before, Settings, Cache and the cache form answered with a plain-text 403. The other admin-only console pages already redirect an IAM-user session.
  • GET /console/metrics-data answers an IAM-user session with HTTP 403 and {"__type":"AccessDeniedException","message":"Metrics are visible to admin users only"}. The handler returns the refusal before it reads the metrics store or the in-memory collector. This route is a JSON endpoint, and the dashboard's script parses its response as JSON. So the route keeps the 403 instead of a redirect to an HTML page.
  • The navigation bar shows the Metrics, Settings and Cache links to admins only. identity_label now returns the label together with whether the caller is an admin. So the 38 nav_bar call sites are unchanged.
  • The page no longer sets const accountFilter for IAM users. The dashboard script never read that variable.

Admin sessions get the same payload, pages and navigation bar as before. The only change for them is that the metrics page no longer has the unused accountFilter variable.

The architecture guide, admin guide, getting started guide and security model now say that the dashboard is admin only and covers every account. The admin guide also lists Settings and Cache as admin only. The upgrade manual has a v0.1.15 entry for the change. The demo manual says to open the dashboard as the admin user. AGENTS.md's /metrics example now passes admin credentials, which that endpoint requires.

Merging with #704 and #734

This PR and #704 (console session revocation) conflict only in the imports of crates/server/src/console/pages/mod.rs. This PR adds use crate::console::html;, and #704 adds use crate::console::session::SessionCheck;. Keeping both lines resolves it. Before the Settings and Cache change, a trial merge with #704 passed 20 tests in cargo test -p extenddb-server --lib console. Against a SQLite build of that merged tree, tests/test_console_metrics_admin_only.py passed 7 cases. The Settings and Cache change adds no conflict with #704.

#734 (console account pages) is on main. It added a sentence to docs/manuals/10-security-model.md: until this PR merges, /console/metrics-data serves every console session the table metrics of every account. 791ae9a removes that sentence. In the same file, #718 rewrote the login rate limiting line next to this branch's new line. 61a5800 merges main and keeps both lines.

Why

/console/metrics-data checked only that the caller had a console session, then returned the rows of the metrics table. That table has no account column, and the store query filters on time, table name and metric only. So any IAM user who could log in to the console read the table names, index names, operations, request counts and latencies of every account. The account id that the page set for IAM users stayed in the browser and did not limit what the server sent.

Reproduction on main's console code (f49c5f823), SQLite backend, 2026-11-01:

  1. Create an account with an IAM user that has a console password. The account has no tables.
  2. Log in as that user at /console/login and request /console/metrics-data.
  3. The response is HTTP 200 and carries 18 distinct TableName values. All of them are tables of other accounts on the server. ?window=LastHour and ?window=AllTime return the same 18 names.

The same steps on this branch return HTTP 403 with the body above and no table names. /console/metrics returns a redirect to /console.

Testing done

New tests:

  • crates/server/src/console/pages/metrics_pages_tests.rs, 5 tests:
    • An IAM-user session gets 403 from the data feed, with no table name, index name or operation in the body. The fake store records no read.
    • An admin gets both accounts' rows, their index dimension and the latency segments.
    • The page redirects an IAM-user session. An admin gets the page without accountFilter.
    • An admin's navigation bar has the Metrics, Settings and Cache links in that order, between Accounts and the spacer. An IAM user's bar has none of them.
  • tests/test_console_metrics_admin_only.py, 13 cases against a server. Two accounts each create a table and send it reads and writes. The fixture waits until the admin's feed shows both tables, which takes up to two flush intervals of 60 seconds. Then:
    • The IAM user of the second account gets 403 and no table names from the feed under four query strings.
    • The Metrics, Settings and Cache pages redirect that user. The cache form redirects the user too, with the session's own CSRF token and with a wrong one.
    • The user's navigation bar has none of the three links. The admin's bar has all three.
    • The admin gets the Metrics page with both tables, the Settings page and the Cache page.

Commands run before the Settings and Cache change:

  • cargo +1.97.0 fmt --all -- --check: clean.
  • cargo +1.97.0 clippy --locked -p extenddb-server --all-targets -- -D warnings: clean. With -W clippy::pedantic the crate has 499 warnings, and none of them are in the changed files.
  • cargo test --locked -p extenddb-server on Rust 1.99.0 and on 1.88.0: 242 passed on each.
  • cargo check --locked --workspace --all-targets: clean.
  • pytest against a SQLite debug build and against a PostgreSQL debug build. On each, tests/test_console*.py passed 42 cases, including the 7 new cases. tests/test_metrics.py with tests/test_cross_account_isolation.py passed 23.

Commands on e28d8db, which adds the Settings and Cache change:

  • cargo +1.97.0 fmt --all -- --check: clean.
  • cargo +1.97.0 clippy --locked -p extenddb-server --all-targets -- -D warnings: clean.
  • cargo +1.97.0 test --locked -p extenddb-server --lib console: 8 passed.
  • pytest against a SQLite debug build: tests/test_console_metrics_admin_only.py passed 13 cases. With the same server build, the other tests/test_console*.py files passed 35.

Negative controls, each restored with git afterwards. Before the Settings and Cache change:

  • Data feed check removed: the Rust 403 test fails. The 4 Python feed cases fail with HTTP 200 and the first account's table in the body.
  • Page check removed: the Rust redirect test fails.
  • Metrics link shown to every session: the Rust nav bar test fails.
  • main's console code: all 7 Python cases fail.

On e28d8db:

  • The three admin links moved after the spacer: the Rust nav bar test fails.
  • The cache form checks the CSRF token before it checks for an admin: the Python case with the wrong token fails.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

On 5d885b4, CI ran the workspace tests on Rust 1.88.0 and on stable, and the integration suites on all four backends. All of them passed. Clippy ran with -D warnings, without the pedantic group. CI runs again on 791ae9a, after the merge of main.

ADR / RFC: n/a

Breaking changes

IAM users lose the console metrics dashboard. The page redirects them to /console, the data feed returns HTTP 403, and the navigation bar no longer links it. The Settings page, the Cache page and the cache form now redirect an IAM-user session to /console, where they answered HTTP 403 before. The navigation bar no longer links Settings and Cache for IAM users. Admin sessions and the admin-authenticated /metrics endpoint are unchanged.

Not covered by this change


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

The console metrics feed at /console/metrics-data returned the rows of the
shared metrics table to any console session. That table has no account
column, so an IAM user of one account read the table names, index names,
operations, request counts and latencies of every account. The page put an
IAM user's account id in a script variable that no script read, and the
server sent every row.

The metrics page now redirects an IAM-user session to the console
dashboard, and the data feed answers it with HTTP 403 and no metric data
before the metrics store or the in-memory collector is read. Admin sessions
get the same page and payload as before. The navigation bar shows the
Metrics link to admins only, and the unused account variable is removed.
Two accounts each create a table and send it reads and writes. Once the
metrics flush has written both tables' rows, as the admin's feed shows, an
IAM user of the second account gets HTTP 403 and no table names from
/console/metrics-data under four query strings, is redirected from
/console/metrics, and has no Metrics link in the navigation bar. The admin
gets the page and both accounts' tables.

The module waits up to three minutes for the flush, so it carries the slow
marker, like the TTL tests that wait.
The architecture guide, admin guide, getting started guide and security
model say the metrics dashboard serves admins and covers every account. The
upgrade manual records the v0.1.15 behaviour change: IAM users no longer
get the dashboard or its data feed. The demo manual says to open the
dashboard as the admin user.
…orrect comments

The fake metrics store's rows now carry an index name. The IAM-user 403
test checks that the refusal does not contain it, and the admin test
checks that the admin's feed reports it as a GlobalSecondaryIndexName
dimension. With no index name in the rows, the first check could not
fail.

The router comments for the metrics page and its data feed, and the
comment in the cross-account isolation test, now say both serve admin
sessions only. The admin guide's console feature list says an IAM-user
session is redirected to the console home page, /console. The metrics
example in AGENTS.md passes admin credentials, which /metrics requires.
Comment thread crates/server/src/console/html.rs Outdated
<a href="/console/accounts">Accounts</a>
<a href="/console/metrics">Metrics</a>
<a href="/console/accounts">Accounts</a>{metrics_link}
<a href="/console/settings">Settings</a>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's an inconsistent refusals for IAM users.

  • Metrics page: redirects to /console.
  • Settings and Cache: plain-text 403 ("Settings are visible to admin users only"), and they're still linked in the navigation bar.
  • Metrics data endpoint: JSON 403.

Three admin-only pages now refuse in three different ways, and the navigation bar hides only one of them. We can hide all admin-only links in the nav bar from IAM users.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. e28d8db hides the Metrics, Settings and Cache links from IAM users. All three pages and the cache form now redirect an IAM-user session to /console through one check, require_admin. /console/metrics-data keeps its JSON 403, because the dashboard's script fetches that route and parses the response as JSON.

The branch conflicts with main in one docs file, so CI has not run on this commit yet. The console unit tests and tests/test_console*.py pass locally against a SQLite build.

The navigation bar showed IAM users the Settings and Cache links, and
those pages answered them with a plain-text 403. The Metrics page
redirected them to /console. The bar now leaves out the Metrics,
Settings and Cache links for an IAM-user session. The three pages and
the cache form refuse that session through one helper, require_admin,
which redirects it to /console. The other admin-only console pages
already redirect an IAM-user session.

The metrics data feed still answers an IAM-user session with HTTP 403
and a JSON body, because the dashboard's script parses that response
as JSON.
hitesh96db
hitesh96db previously approved these changes Nov 4, 2026

@hitesh96db hitesh96db left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

#718 rewrote the login rate limiting bullet in docs/manuals/10-security-model.md and added a section after it, which conflicted with the admin-only metrics bullet in this PR. 61a5800 merges main and keeps both: the bullet and section from #718, then this PR's bullet. The merge changes no code.

On the merge, cargo +1.97.0 clippy --locked --all-targets -- -D warnings is clean and cargo +1.88.0 test --locked -p extenddb-server passes. The push dismissed the approval, so the PR needs a new one before it can go into the queue.

#734 added a sentence that says /console/metrics-data serves every console
session the metrics of every account until this pull request merges. This
pull request makes the feed admin only, so the sentence is false once it
merges. The admin-only metrics bullet in the same section states the new
behaviour.
@LeeroyHannigan

Copy link
Copy Markdown
Collaborator Author

791ae9a removes the sentence that #734 added to docs/manuals/10-security-model.md about /console/metrics-data. The sentence says the feed serves every console session until this PR merges. The merge in 61a5800 kept it, and the description says this PR removes it. The commit changes that one paragraph and no code.

@hitesh96db
hitesh96db added this pull request to the merge queue Nov 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Nov 5, 2026
@LeeroyHannigan
LeeroyHannigan added this pull request to the merge queue Nov 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026
@LeeroyHannigan
LeeroyHannigan added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit e2757cb Nov 5, 2026
39 checks passed
@LeeroyHannigan
LeeroyHannigan deleted the fix/console-metrics-admin-only branch November 5, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working release-blocker Must ship for its milestone's release security v1 Needed for the 1.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants