Remove the generated machine token and split it into write and read tokens - #147
Merged
Merged
Conversation
lewispb
changed the title
lewispb
force-pushed
the
security/machine-tokens-and-proxy-origins
branch
from
September 7, 2026 07:43
b6c8c4b to
ff644a0
Compare
There was a problem hiding this comment.
🟡 Changes recommended
UI-host collisions and inaccurate deployment and vulnerability guidance must be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Strengthens proxy security by separating machine credentials, isolating upstream UIs onto dedicated origins, and updating deployment safeguards.
Changes:
- Splits OTLP-write and metrics-read authentication.
- Isolates Prometheus and Alertmanager UIs with stricter framing and request gates.
- Updates generator templates, tests, deployment configuration, and migration guidance.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
UPGRADING.md |
Adds 0.4-to-0.5 migration steps. |
test/models/upright/site_test.rb |
Tests peer read-token usage. |
test/lib/upright/configuration_test.rb |
Tests tokens and UI-origin mapping. |
test/integration/proxy_csrf_test.rb |
Tests revised Fetch-Metadata policy. |
test/integration/prometheus_proxy_controller_test.rb |
Tests Prometheus isolation and authorization. |
test/integration/alertmanager_proxy_controller_test.rb |
Tests Alertmanager isolation and authorization. |
README.md |
Documents tokens, UI origins, DNS, and deployment. |
lib/upright/site.rb |
Uses the metrics-read token for peers. |
lib/upright/engine.rb |
Validates machine tokens at boot. |
lib/upright/configuration.rb |
Defines split tokens and UI origins. |
lib/generators/upright/install/templates/upright.rb |
Removes generated token literals. |
lib/generators/upright/install/templates/pre-deploy |
Adds deployment token validation. |
lib/generators/upright/install/templates/kamal_secrets |
Adds Kamal secret references. |
lib/generators/upright/install/templates/docker-compose.yml |
Updates Prometheus. |
lib/generators/upright/install/templates/deploy.yml |
Pins images and configures secrets. |
lib/generators/upright/install/templates/content_security_policy.rb |
Documents UI frame sources. |
lib/generators/upright/install/install_generator.rb |
Installs secrets and deployment hook. |
docs/sites.md |
Documents site token and origin behavior. |
config/routes.rb |
Separates admin and UI-origin routes. |
CHANGELOG.md |
Records security changes. |
app/views/upright/prometheus_proxy/show.html.erb |
Removes the old inline frame. |
app/views/upright/framed/prometheus.html.erb |
Adds the isolated Prometheus frame. |
app/views/upright/framed/alertmanager.html.erb |
Adds the isolated Alertmanager frame. |
app/views/upright/alertmanager_proxy/show.html.erb |
Removes the old inline frame. |
app/helpers/upright/application_helper.rb |
Builds paired UI-origin URLs. |
app/controllers/upright/prometheus_proxy_controller.rb |
Enforces OTLP-specific authentication. |
app/controllers/upright/framed_controller.rb |
Serves admin framing pages. |
app/controllers/upright/alertmanager_proxy_controller.rb |
Applies universal upstream-path denial. |
app/controllers/concerns/upright/proxy_authentication.rb |
Implements split auth, CSRF gating, and framing headers. |
Review details
Suppressed comments (1)
lib/upright/configuration.rb:186
- The generated UI hostname can collide with an existing site code, but site loading currently accepts arbitrary codes. For example, a site named
prometheusshadows the global Prometheus UI host, while a site namedprometheus-amsshadows the UI host forams; route ordering then serves the wrong admin/machine path or proxy UI. Reject site codes that intersect the generated proxy UI subdomains at startup (and document this upgrade constraint).
def video_storage_dir
@video_storage_dir || Rails.root.join("storage", "playwright_videos")
- Files reviewed: 20/20 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| copy_file "kamal_secrets", ".kamal/secrets" | ||
| end | ||
|
|
||
| copy_file "pre-deploy", ".kamal/hooks/pre-deploy", mode: :preserve |
Comment on lines
+499
to
+500
| # Pinned by digest; 3.5.2 and later fix CVE-2026-40179 (stored XSS in the web UI) | ||
| image: prom/prometheus:v3.5.5@sha256:332c2f43e7e389d74d3893b55bb02fbbd684208e681eeb604641d5d769c0fe2a |
Comment on lines
+37
to
+39
| During a rolling deploy, sites still on 0.4 read peers with the OTLP token and | ||
| receive 401 from sites already on 0.5. The hourly rollup retries on its next | ||
| run, so finish the deploy within the hour or expect one skipped run. |
Comment on lines
+90
to
+91
| # 3.5.2 and later fix CVE-2026-40179, stored XSS through metric names in the | ||
| # web UI, which the OTLP receiver below can ingest. |
…okens
The install template wrote config.proxy_token = ENV.fetch("PROMETHEUS_OTLP_TOKEN",
"<random>") into the generated app, so every generated app committed a token
that production accepted and that authorized both metric writes and proxy
reads. The generated deploy also ran Prometheus 3.2.1, whose web UI has a stored
XSS through metric names (CVE-2026-40179) that the OTLP receiver ingests.
Together those let anyone who could read a generated repository write a crafted
metric and run script in the admin's browser.
Tokens. The template reads both tokens from the environment with no fallback,
and config.proxy_token= raises with directions. PROMETHEUS_OTLP_TOKEN
(config.otlp_token) authorizes only the OTLP write route; METRICS_READ_TOKEN
(config.metrics_read_token) authorizes only GET and HEAD on the proxies. Outside
development and test the app refuses to boot when either is missing or both are
the same value. Peer reads in Upright::Site use the read token.
Deploy. Prometheus is pinned to 3.5.5 by digest and Alertmanager to 0.28.1 by
digest; the development compose file moves to 3.5.5. The generator adds the
tokens and ADMIN_PASSWORD to .kamal/secrets and installs .kamal/hooks/pre-deploy,
which stops kamal deploy before the build when either token is empty or both
are the same.
Docs cover the tokens and a 0.4 to 0.5 upgrade guide.
lewispb
force-pushed
the
security/machine-tokens-and-proxy-origins
branch
from
September 7, 2026 07:47
ff644a0 to
60b8092
Compare
There was a problem hiding this comment.
🟢 Approval recommended
Token separation is consistently enforced across configuration, proxy authorization, deployment templates, documentation, and tests.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
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.
Resolves the finding in this card: the install template committed a production-capable token into the generated app, and the generated deploy ran a Prometheus whose web UI has a stored XSS (CVE-2026-40179).
Changes
Token removed from source.
lib/generators/upright/install/templates/upright.rbno longer setsconfig.proxy_tokenwith aSecureRandomfallback. Both tokens are read from the environment.config.proxy_token=raisesUpright::ConfigurationErrorwith directions, so a 0.4 initializer fails at boot instead of keeping one token for both jobs.Token split.
PROMETHEUS_OTLP_TOKEN(config.otlp_token) authorizes onlyPOST /prometheus/api/v1/otlp/v1/metrics.METRICS_READ_TOKEN(config.metrics_read_token) authorizes onlyGETandHEADon the proxies.Upright::Configuration#verify_machine_tokensruns at boot outside development and test and refuses a missing token or two equal tokens. A blank configured token matches nothing. Peer reads use the read token.Deploy template. Prometheus
v3.5.5pinned by digest, Alertmanagerv0.28.1pinned by digest, compose file atv3.5.5. The generator appendsPROMETHEUS_OTLP_TOKEN,METRICS_READ_TOKENandADMIN_PASSWORDto.kamal/secrets(or creates it) and installs.kamal/hooks/pre-deploy, which checks the resolved secrets Kamal passes to pre-deploy hooks and stops the deploy, after the build and before any container is replaced, when either token is empty or both are equal. Kamal otherwise deploys an empty secret and the failure appears only at the health check.Docs. README sections "Machine tokens" and "Secrets",
docs/sites.md, a "From 0.4 to 0.5" section inUPGRADING.md, and the changelog.Operator impact
config.proxy_tokenin an existing initializer raises at boot.METRICS_READ_TOKENmust be added as a Kamal secret.Not included
Verification
bin/rails test: 395 runs, 0 failures. New coverage: each token is refused for the other's job, a blank configured token matches nothing, the boot check, andconfig.proxy_token=raising.bin/rubocopandbin/brakemanclean..kamal/secrets; hook script exercised with the tokens set, missing and equal.