Skip to content

server: add --cors-* options - #25655

Merged
ServeurpersoCom merged 5 commits into
masterfrom
xsn/server_cors_args
Jul 14, 2026
Merged

ServeurpersoCom merged 5 commits into
masterfrom
xsn/server_cors_args

Conversation

@ngxson

@ngxson ngxson commented Jul 14, 2026 •

Copy link
Copy Markdown
Collaborator

Overview

Traditionally, llama-server always enable CORS * with no way to disable it. This was fine because llama-server endpoints were stateless, read-only and default config doesn't expose any sensitive data. The only risk was DoS which is not considered a vulnerability per llama.cpp's policy.

However, with the addition of --agent mode (built-in tools), server can now expose API that allow read/write arbitrary files. If combine with the relaxed CORS, it may allow malicious pages to access local file from server API.

This PR introduce 4 flags, inspired by vllm:

  • --cors-origins
  • --cors-methods
  • --cors-headers
  • --(no-)cors-credentials

By default:

  • the relaxed profile is used: credentials are allowed and Origin header is reflected back (no breaking changes), but a warning will show when server starts
  • if server tools are enable, CORS origin will be limited to localhost (can override via --cors-origins)

Recommendations

Depending on use case

  • Public deployment:
    • highly recommended setting an API key
    • explicit --cors-origins is optional
    • using llama-server behind a reverse proxy to mitigate DoS-related issues
  • Local (trusted) network deployment:
    • recommend explicitly set --cors-origins to origin server address
  • Local deployment (same machine):
    • recommend setting --cors-origins localhost (auto set via --agent)

Requirements

@ngxson
ngxson marked this pull request as ready for review July 14, 2026 11:07
@ngxson
ngxson requested review from a team as code owners July 14, 2026 11:07
@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Memo to myself: I'm going to test/merge this one

@pwilkin

pwilkin commented Jul 14, 2026

Copy link
Copy Markdown
Member

@ngxson Definitely should have a separate documentation section in the server notes, at least with what you just wrote in the PR description.

@ServeurpersoCom ServeurpersoCom self-assigned this Jul 14, 2026
@ServeurpersoCom

ServeurpersoCom commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

@ngxson Definitely should have a separate documentation section in the server notes, at least with what you just wrote in the PR description.

I think I can commit to the branch if you approve this proposal, @pwilkin?

doc proposal :

diff --git a/tools/server/README.md b/tools/server/README.md
index 9c8b32dcc..84132b5d7 100644
--- a/tools/server/README.md
+++ b/tools/server/README.md
@@ -329,6 +329,22 @@ The server includes a set of built-in tools that enable the LLM to access the lo

 To use this feature, start the server with `--tools all`. You can also enable only specific tools by passing a comma-separated list: `--tools name1,name2,...`. Run `--help` for the full list of available tool names.

+### CORS
+
+By default the server reflects any `Origin` header back with credentials allowed. This matches the old, always-on `*` behavior and is fine as long as the server only exposes stateless, read-only endpoints.
+
+Enabling `--tools` or `--agent` exposes file read/write over the API, so in that case `--cors-origins` defaults to `localhost` instead: only pages served from localhost can reach the server. Pass `--cors-origins` explicitly to override either default.
+
+Recommended `--cors-origins` setting, depending on where the server runs:
+
+| Deployment | Recommendation |
+| ---------- | --------------- |
+| Public | set an API key, put the server behind a reverse proxy, `--cors-origins` optional |
+| Local network | set `--cors-origins` to your frontend's origin |
+| Same machine | `--cors-origins localhost` (default once `--agent` is set) |
+
+Related flags: `--cors-origins`, `--cors-methods`, `--cors-headers`, `--cors-credentials` / `--no-cors-credentials`. Background and rationale: [#25655](https://github.com/ggml-org/llama.cpp/pull/25655).
+
 ## Build

 `llama-server` is built alongside everything else from the root of the project

CORS

By default the server reflects any Origin header back with credentials allowed. This matches the old, always-on * behavior and is fine as long as the server only exposes stateless, read-only endpoints.

Enabling --tools or --agent exposes file read/write over the API, so in that case --cors-origins defaults to localhost instead: only pages served from localhost can reach the server. Pass --cors-origins explicitly to override either default.

Recommended --cors-origins setting, depending on where the server runs:

Deployment Recommendation
Public set an API key, put the server behind a reverse proxy, --cors-origins optional
Local network set --cors-origins to your frontend's origin
Same machine --cors-origins localhost (default once --agent is set)

Related flags: --cors-origins, --cors-methods, --cors-headers, --cors-credentials / --no-cors-credentials. Background and rationale: #25655.

@ServeurpersoCom ServeurpersoCom left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

What's I tested :

  • default (*, credentials) behavior unchanged, startup warning fires

  • --agent / --tools switches default to localhost, explicit override works

  • reflects localhost / 127.0.0.1 / [::1], rejects everything else

  • /cors-proxy forwards only explicit proxy headers

  • test_security.py: 17/17 passed

  • adversarial: null origin, subdomain suffix, decimal/mapped IP, case, trailing dot all rejected

  • minor: "http://evil.com@localhost" accepted, host correctly parses as localhost per URL syntax, not exploitable: browsers never put userinfo in the Origin header (RFC 6454), no real request can carry this value

@ServeurpersoCom
ServeurpersoCom merged commit 6e52db5 into master Jul 14, 2026
30 of 31 checks passed
@leolebkuchen leolebkuchen mentioned this pull request Jul 22, 2026
2 tasks done
@LsnavTech LsnavTech mentioned this pull request Jul 27, 2026
linhdmn added a commit to FreePeak/LeanKG that referenced this pull request Oct 21, 2026
… store (#556)

* fix(dashboard): give the dashboard verb one flag set

`leankg dashboard --addr 127.0.0.1:9701 --no-open --format text` died
with "flag provided but not defined: -addr" — the invocation AGENTS.md
and docs/plan-dashboard.md both document.

The verb had two flag sets: the web server's (--addr, --no-open,
--allow-remote, --token) and the usage tables' (--project, --since,
--format). Which one ran was decided by scanning argv for "--format",
so the whole argv went to the tables' set and the web flags died in it.
The usage output was the same defect seen from the other side: it
listed only the tables' flags, because that was the set that answered.

parseDashboardFlags now declares all seven flags in ONE set, and
cmdDashboard reads the parsed values: fs.Visit on "format" records
that --format was given, and that is the mode. The argv scan is gone —
a scan of the raw arguments is what let the two sets disagree about
the same command. dashboard.CheckListen is untouched, so a non-loopback
listen still needs --allow-remote with a --token.

* fix(dashboard): name the dashboard mode instead of a formatSet bool

The one flag set already decided the mode from the parsed values, but the
decision rode on a `formatSet` bool that only meant "was --format given".
The mode is a first-class thing the verb runs, so it is now named:
dashboardModeWeb (the default, DS-20) and dashboardModeTables, and
cmdDashboard switches on it.

The web mode is the zero value, so a mode that was never set is never
silently the tables.

Two tests close the gaps in the acceptance:

- TestDashboardFormatJSONEmitsJSON runs `leankg dashboard --format json`
  end to end and requires the JSON payload (totals, by_tool, by_day,
  by_project, patterns) and NO web-server banner.
- TestBareDashboardTakesTheWebPath asserts that a bare `leankg dashboard`
  parses to the web mode. It asserts on the parsed flags rather than
  running the verb: the web path binds a port, opens a browser and
  serves until a signal, so a bare run in a test would hang and leave a
  listener behind.

* test(dashboard): cover the JSON path and the bare-invocation web mode

Two gaps in the acceptance of the one-flag-set fix:

- `--format json` reaching the table path and emitting the JSON payload
  was unproven. TestDashboardFormatJSONEmitsJSON runs it end to end and
  requires exit 0, a stdout that parses as JSON carrying totals, by_tool,
  by_day, by_project and patterns, and no web-server banner.
- "a bare `leankg dashboard` still reaches the WEB path" was implied by
  a flagSet bool rather than asserted. TestBareDashboardTakesTheWebPath
  asserts the web mode on the PARSED flags. It deliberately does not run
  the verb: the web path binds a port, opens a browser and serves until a
  signal, so a bare run in a test would hang and leave a listener behind.

dashboard_flags_test.go is now black box — it drives the built binary and
reads what the process printed — so it also compiles against the pre-fix
source. With origin/main's dashboard_web.go and metrics.go checked out and
only that file kept, TestDashboardAcceptsTheWebFlagsWithTheFormatFlag
fails with the measured "flag provided but not defined: -addr" (exit 2).

The docs sections name the new tests and the mode enum.

* fix(cli): metrics, dashboard and migrate refuse a project that has no store

Opening a missing store read-write CREATES it (file, schema and all), so
three verbs materialised a store for a project nobody indexed:

    leankg metrics                    -> CREATES the store
    leankg dashboard --format json    -> CREATES the store
    leankg migrate --project DIR      -> CREATES the store

The blast radius is repo-wide: the portfolio registry holds a row for
/private/tmp, so a writer sweep materialises /private/tmp/.leankg, and
that one stray directory BREAKS THE WHOLE REPO TEST SUITE
(internal/projectcfg.FindProjectRoot walks up and latches onto the
first ancestor with .leankg or leankg.yaml).

The three verbs now ask maintenanceStoreExists - the CLI's one
definition of "this project has a store" - before opening one, and
refuse with the diagnosis ok store=/Users/linh.doan/work/harvey/freepeak/leankg/.leankg/leankg.db elements=11487 files=1094 watermark(seq=46186 at=1792518521) prints for the same state:
"FAIL not indexed: no store at ..." plus the indexed .: files=1094 elements=11622 relationships=28104 skipped=0
next: leankg install --target <client>  (claude-code | cursor | codex | gemini | opencode | omp | xdev)
embed skipped: embed: sidecar "llama-server" exited during startup: exit status 1; stderr: 0.00.001.920 I srv  llama_server: initializing ...
0.00.243.584 I cmn  common_param: common_params_print_info: verbosity = 3 (adjust with the `-lv N` CLI arg)
0.00.243.587 W srv  llama_server: embeddings enabled with n_batch (2048) > n_ubatch (512)
0.00.243.587 W srv  llama_server: setting n_batch = n_ubatch = 512 to avoid assertion failure
0.00.245.575 W srv  llama_server: security: no API key is set and CORS allows all origins (see ggml-org/llama.cpp#25655)
0.00.245.800 E srv         start: couldn't bind HTTP server socket, hostname: 127.0.0.1, port: 8080
0.00.245.802 I srv    operator(): operator(): cleaning up before exit...
0.00.245.990 E srv  llama_server: exiting due to HTTP server error; port 8080 is held by another process that does not serve /v1/embeddings — set LEANKG_EMBED_SIDECAR_PORT to a free port, or attach to it with LEANKG_EMBED_BASE_URL fix,
exit 2.

* test(cli): pin that metrics, dashboard and migrate refuse an unindexed project

The regression test for the store-creation guard, plus the one existing
test that pinned the old behaviour.

TestWriterVerbsRefuseAnUnindexedProject runs each verb against a
project directory with NO store and asserts the consumer-visible
contract: exit 2, "FAIL not indexed" on stderr, an EMPTY stdout (the
payload contract is not corrupted), and NO store on disk — the last
assertion is the one that proves the defect, because before the fix
opening a missing store read-write CREATED it.

TestWriterVerbsStillRunOnAnIndexedProject is the other half: a project
that HAS a store keeps working, so a guard that refused everything
cannot pass the test above.

TestMigrateCLI pinned the old behaviour (migrate on a fresh project
creating the store and applying the plan). The state it means to test
is "a store whose migration ledger is empty" — what a project indexed
by an older leankg looks like — so it now seeds that state explicitly
with seedUnmigratedStore.

Verified to FAIL on origin/main (all four subtests: exit 0, and a
store left on disk) and PASS on this branch.

* docs(prd): record the cmd verb-surface guard (metrics, dashboard, migrate)

The new section lands in docs/prd.md before the v4.33.0 anchor and in
docs/prd-task-tracker.md before the live-Todo anchor, with the
measurement, the rule, the refusal shape, the semantics decided per
verb, and the repo-wide blast radius (`mkdir -p /private/tmp/.leankg`
breaks the whole test suite, because FindProjectRoot walks up).

The status -wal side effect is recorded as measured and deliberately
left alone: the store is opened with query_only(ON) and the sidecars
appear because the DATABASE FILE is in WAL mode; avoiding them would
need immutable=1, which serves stale data for a live writer.

* test(cmd): seed the project store the dashboard tables path reads

The tables path opens the project store read-write, and a read-write open
CREATES a missing store. So the flag-routing tests were asserting a
side effect nobody asked for: they materialised a .leankg in a throwaway
directory. The fix that refuses a writer verb against an unindexed project
(PR #556) would have made them fail for the wrong reason.

Seed the store at the path the child process is pointed at, so the tests
keep testing what they are about: ONE FlagSet, --format selecting the
tables, and the web flags surviving alongside it.

* docs(prd): move the cmd-verb-surface tracker section to a free anchor
thexa4 added a commit to DelftSolutions/vscode-llama-copilot that referenced this pull request Nov 14, 2026
llama-server reflects any Origin by default, so any website could read
the local server's responses. Start the managed server with
LLAMA_ARG_CORS_ORIGINS=localhost (ggml-org/llama.cpp#25655). The env
var is ignored by older builds instead of failing on an unknown flag,
and --cors-origins in server.extraArgs still overrides it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants