Skip to content

nginx: Support query params & fix OPCS-4 port conflict #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
phoevos merged 2 commits into from
Jan 14, 2026
+11 −3
Merged

nginx: Support query params & fix OPCS-4 port conflict #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
417b81c
nginx: Include query params when passing to upstream
phoevos Jan 9, 2026
3c3ca98
nginx: Use new port for OPCS-4 to avoid ICD-10 conflict
phoevos Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docker/nginx/etc/nginx/nginx.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ http {
include cors.conf;
resolver 127.0.0.11 valid=30s;
set $upstream $service:8000;
set $path $subpath;
if ($subpath = "") {
set $path /;
}
set $query_string_part "";
if ($is_args) {
set $query_string_part $is_args$args;
}

# FIXME: Access web interfaces (e.g. Grafana, MLflow) through subpaths on the proxy.
# The following services only work when accessed directly through their respective APIs.
Expand Down Expand Up @@ -76,7 +84,7 @@ http {
set $upstream $service:9090;
}

proxy_pass http://$upstream$subpath;
proxy_pass http://$upstream$path$query_string_part;
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The proxy_pass target host is built from the unvalidated $service path segment (captured in the location regex and then used to construct $upstream), which allows clients to control the upstream host for /cms/<service>/... requests. An attacker can send requests like /cms/attacker-controlled-host/some/path?x=y to force nginx to proxy arbitrary HTTP traffic to internal or external hosts on the configured ports, effectively creating an SSRF/open-proxy primitive. To mitigate this, restrict $service to a strict allowlist of known backends (e.g. via dedicated location blocks or a map) instead of passing arbitrary user-controlled values into proxy_pass.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@phoevos phoevos Jan 12, 2026

Choose a reason for hiding this comment

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

This comment is interesting but not directly relevant to this PR, but what's described has in fact been the case since I introduced dynamic routing through the proxy, the whole point of which was to move away from strictly defined targets so that we can proxy to models deployed dynamically through the CMG.

Rationale behind dynamic routing through proxy

When running in airgapped environments, a proxy server is usually configured to handle all traffic deemed to be targeting resources outside of the local network. This also affects containerised applications using Docker service names for communication. For these to work, we explicitly add the names of the expected targets to the containers' no_proxy list. However, this can only work when we know the names of all potential targets in advance (e.g. a backend server that only talks to a database and an object store). In the case of the CogStack Model Gateway, which allows the dynamic deployment of user-defined model servers, the list of targets can change dynamically. For that reason, instead of keeping track of all servers and attempting to maintain an up-to-date no_proxy list, we choose to proxy all requests through a single service, the name of which we can safely add to the CMG containers' no_proxy list.

Risk

In airgapped setups like our production environments, the risk posed by the unrestricted proxy pass is minimal. Any traffic exiting the cluster (outbound) is restricted by the configured Squid proxy, meaning that only local services can be targeted through the proxy. With that in mind, if an attacker is able to create arbitrary containers inside the Docker network, they hardly need the proxy to compromise our servers.

On the other hand, in environments with unrestricted outbound traffic, this can indeed be an actual security concern. For this reason, we can look into moving the dynamic routing logic to a separate internal proxy managed by the CMG project and revert to solely using the strictly configured upstreams here. In any case, that should be part of a different PR.


proxy_redirect http://$upstream$subpath $scheme://$host/cms/$service$subpath;
proxy_redirect http://$upstream/ $scheme://$host/cms/$service/;
Expand Down
4 changes: 2 additions & 2 deletions docker/nginx/etc/nginx/sites-enabled/medcat-opcs4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
server {
listen 28181 ssl http2 default_server;
listen [::]:28181 ssl http2 default_server;
listen 28182 ssl http2 default_server;
listen [::]:28182 ssl http2 default_server;
server_name localhost;

add_header Strict-Transport-Security "max-age=31536000" always;
Expand Down