docs: security threat model and review findings
This commit is contained in:
1 parent
ce5ae6caa5
commit
5baf73f38b
2 files changed
+665
-82
No files matched your search
@@ -0,0 +1,456 @@
|
|||||||
|
# Security review findings (S1)
|
||||||
|
|
||||||
|
Review of commit `ce5ae6c` on `feat/full-api-coverage`, 2026-10-14. Scope: every path from an
|
||||||
|
HTTP request or a stdio message to a Gitea call (`transports.py`, `auth.py`, `server.py`,
|
||||||
|
`oauth/*`, `gitea/*`, `rate_limit.py`, `config.py`), audit and logging, `setup_cmd.py`, the
|
||||||
|
Docker image and CI. The threat model is in [security.md](security.md). S2 fixes these
|
||||||
|
findings and adds the negative tests to `tests/security/`.
|
||||||
|
|
||||||
|
Severity: critical, high, medium, low. Reproduction scripts referenced below were run in a
|
||||||
|
scratch directory against the code as committed; each fix lists the negative test that proves
|
||||||
|
it.
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
| Id | Severity | Title |
|
||||||
|
|---|---|---|
|
||||||
|
| SEC-01 | high | Unbounded request body on the unauthenticated consent POST |
|
||||||
|
| SEC-02 | medium | Stored webhook `authorization_header` returned by hook read tools |
|
||||||
|
| SEC-03 | medium | Audit write failures after startup do not stop later calls |
|
||||||
|
| SEC-04 | medium | Settings read a `.env` file from the process working directory |
|
||||||
|
| SEC-05 | medium | Any https redirect URI can be registered; the consent page is the only barrier |
|
||||||
|
| SEC-06 | medium | Fork pull requests run Docker-capable jobs on the shared runner |
|
||||||
|
| SEC-07 | medium | CI actions pinned by mutable tags, one of them receives the registry token |
|
||||||
|
| SEC-08 | low | Rate limiting gaps (`/token`, `/revoke`, IPv6, proxy address) |
|
||||||
|
| SEC-09 | low | OAuth pre-sign-in state can be flushed by unauthenticated floods |
|
||||||
|
| SEC-10 | low | `GITEA_URL` and `PUBLIC_BASE_URL` accept whitespace and control characters; plain http Gitea is silent |
|
||||||
|
| SEC-11 | low | Audit log created with umask defaults; unkeyed hash chain |
|
||||||
|
| SEC-12 | low | Tool audit entries do not record the client, session or source IP |
|
||||||
|
| SEC-13 | low | Token store directory ownership is not verified |
|
||||||
|
| SEC-14 | low | CI cleanup and credential handling on the shared Docker daemon |
|
||||||
|
| SEC-15 | low | `tests/security` missing, so the CI `security` job fails; publish is not gated on it |
|
||||||
|
| SEC-16 | low | No container image vulnerability scan |
|
||||||
|
| SEC-17 | low | Consent POST accepts a missing `Origin` header, unlike ADR 0004 says |
|
||||||
|
| SEC-18 | low | Unvalidated argument names and unknown tool names reach the audit log |
|
||||||
|
| SEC-19 | low | One fixed cookie name for concurrent sign-ins in the same browser |
|
||||||
|
|
||||||
|
No critical findings. Verified as sound (no finding): static bearer comparison, the identity
|
||||||
|
split between static bearer, OAuth and no-auth (including OAuth-only deployments), Sudo
|
||||||
|
handling for OAuth users, path, query and header encoding in the executor, redirect handling,
|
||||||
|
response size and JSON depth bounds, stdio stdout hygiene, error detail exposure, PKCE, code
|
||||||
|
and refresh replay handling, consent CSRF and clickjacking headers, the browser-binding
|
||||||
|
cookie, token store encryption and atomic writes, the setup command's env file handling, and
|
||||||
|
the Dockerfile (pinned digests, non-root, no secrets in layers).
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### SEC-01 (high): unbounded request body on the unauthenticated consent POST
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/callback.py:137-155` (`consent_submit` calls
|
||||||
|
`await request.form()`), routes at `callback.py:169-173`.
|
||||||
|
- Description: the browser routes are custom Starlette routes. Unlike the MCP endpoint and the
|
||||||
|
SDK's `/authorize`, `/token`, `/register` and `/revoke` (all wrapped in
|
||||||
|
`RequestBodyLimitMiddleware`), they have no body limit, and `uvicorn` sets none. Starlette
|
||||||
|
parses `multipart/form-data` with up to 1000 fields of up to 1 MiB each held in memory, and
|
||||||
|
file parts of unlimited size spooled to `/tmp`, which is a RAM-backed tmpfs in the shipped
|
||||||
|
`docker-compose.yml`. The `Origin` check passes when the header is absent, and the CSRF check
|
||||||
|
only runs after parsing. Reproduced: one unauthenticated POST with 500 one-megabyte multipart
|
||||||
|
fields raised the server's RSS from 93 MB to 593 MB; a single 200 MB file part was consumed
|
||||||
|
completely before the 400 answer. The per-IP limit (120 requests per minute) counts requests,
|
||||||
|
not bytes, so a single client can exhaust memory and get the process killed. OAuth mode only.
|
||||||
|
- Fix: wrap the consent and callback routes in `RequestBodyLimitMiddleware` with a small cap
|
||||||
|
(16 KiB is ample: the form has three short fields); answer 415 before reading the body when
|
||||||
|
the content type is not `application/x-www-form-urlencoded`; parse with
|
||||||
|
`request.form(max_files=0, max_fields=8, max_part_size=4096)`.
|
||||||
|
- Negative test (`tests/security/test_oauth_input_bounds.py`): drive the ASGI app with a
|
||||||
|
counting `receive`; a 1 MiB urlencoded POST to `/oauth/consent` gets 413 having read at most
|
||||||
|
the cap plus one chunk; a multipart POST gets 415 having read no body; a valid consent still
|
||||||
|
works.
|
||||||
|
|
||||||
|
### SEC-02 (medium): stored webhook `authorization_header` returned by hook read tools
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/gitea/executor.py:339-356` and `603-625` (only values submitted
|
||||||
|
in the request body are redacted, via `sensitive_body_values` in
|
||||||
|
`src/aegis_gitea_mcp/gitea/encoding.py:387-405`).
|
||||||
|
- Description: the Gitea `Hook` schema includes `authorization_header`, and Gitea fills it on
|
||||||
|
list, get and edit. A create call redacts the header because the caller submitted it, but
|
||||||
|
`gitea_hook_repo_list_hooks`, `gitea_hook_repo_get_hook` and the org, user and admin
|
||||||
|
equivalents (and `gitea_api_request` on hook paths) return stored headers verbatim to the
|
||||||
|
model and the remote client. Reproduced with a mocked `repoListHooks` answer: the header
|
||||||
|
value appears in the result. Pattern masking does not catch `Basic ...` or most `Bearer ...`
|
||||||
|
values.
|
||||||
|
- Fix: redact known secret-bearing response fields in `_seal`, before pattern masking: a
|
||||||
|
non-empty `authorization_header` at any depth becomes `[REDACTED]` (presence stays visible).
|
||||||
|
Keep the set (`_SENSITIVE_RESPONSE_KEYS`) next to `_SENSITIVE_BODY_KEYS` so new fields are
|
||||||
|
added in one place. Credential-issuing operations are unaffected (no hook operation is one).
|
||||||
|
- Negative test (`tests/security/test_secret_echo.py`): list, get and edit hook responses
|
||||||
|
(repo, org, user, admin, and the raw tool on `/repos/o/r/hooks`) containing
|
||||||
|
`authorization_header` never contain the value in `content` or `structured_content`, with
|
||||||
|
`SECRET_DETECTION_MODE` set to `off`, `mask` and `block`.
|
||||||
|
|
||||||
|
### SEC-03 (medium): audit write failures after startup do not stop later calls
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/server.py:414-426` (only obtaining the logger is checked) and
|
||||||
|
`server.py:509-537` (write errors are logged and swallowed);
|
||||||
|
`src/aegis_gitea_mcp/audit.py:104-107`.
|
||||||
|
- Description: the fail-closed check only proves the file could be opened once. Once the
|
||||||
|
singleton exists, a full disk, a revoked mount or any other write error makes every later call
|
||||||
|
run unaudited, indefinitely, while AGENTS.md promises an audit event for every call. The
|
||||||
|
post-execution swallow is intentional for the call that already ran, but nothing stops the
|
||||||
|
next one.
|
||||||
|
- Fix: record audit health. On a write failure set `AuditLogger.healthy = False`; at the start
|
||||||
|
of `call_tool`, refuse with `audit_unavailable` while unhealthy, after one probe write
|
||||||
|
(a small `audit_probe` security event) fails. Alternatively write a `tool_invocation_started`
|
||||||
|
entry before execution and refuse when it fails. Either way, document the choice in
|
||||||
|
`docs/audit-logging.md`.
|
||||||
|
- Negative test (`tests/security/test_audit_fail_closed.py`): after one successful call, make
|
||||||
|
the log file's `write` raise `OSError`; the next call returns `audit_unavailable` and the
|
||||||
|
mocked Gitea receives no request; when writes work again, calls resume.
|
||||||
|
|
||||||
|
### SEC-04 (medium): settings read a `.env` file from the process working directory
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/config.py:109-114` (`env_file=".env"`).
|
||||||
|
- Description: pydantic-settings loads `./.env` for every setting not present in the
|
||||||
|
environment. Claude Code and other clients start stdio servers with the project directory as
|
||||||
|
the working directory, so a cloned repository can plant a `.env`. Reproduced: with
|
||||||
|
`GITEA_URL` and `GITEA_TOKEN` in the environment, a world-readable `.env` in the working
|
||||||
|
directory set `SECRET_DETECTION_MODE=off`, `EXPOSE_ERROR_DETAILS=true` and
|
||||||
|
`AUDIT_LOG_PATH=/dev/null` (all calls then go unaudited, and the startup audit check passes).
|
||||||
|
A client configuration without `GITEA_URL` in the environment would also send the token to an
|
||||||
|
attacker-chosen `GITEA_URL`.
|
||||||
|
- Fix: honour `./.env` only when it is a regular file (not a symlink) owned by the current user
|
||||||
|
and not readable or writable by group or others (the mode `setup` writes, 0600); otherwise
|
||||||
|
ignore it and print one warning to stderr. Add an explicit `--env-file` option (and
|
||||||
|
`AEGIS_ENV_FILE`) for other locations. Document it in `docs/configuration.md`.
|
||||||
|
- Negative test (`tests/security/test_env_file.py`): a 0644 `.env` in the working directory
|
||||||
|
with `SECRET_DETECTION_MODE=off` and `AUDIT_LOG_PATH=/dev/null` is ignored (mode stays
|
||||||
|
`mask`, the audit path stays under `DATA_DIR`); a symlinked `.env` is ignored; a 0600 `.env`
|
||||||
|
owned by the user is honoured; environment values still win.
|
||||||
|
|
||||||
|
### SEC-05 (medium): any https redirect URI can be registered; the consent page is the only barrier
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/provider.py:99-115` (`redirect_uri_problem`),
|
||||||
|
`provider.py:306-343` (`register_client`), `src/aegis_gitea_mcp/oauth/callback.py:109-135`.
|
||||||
|
- Description: dynamic client registration is open to anyone and accepts any https redirect
|
||||||
|
URI and any `client_name`. Gitea remembers consent for our single application, so the only
|
||||||
|
thing between a phishing link and a victim's full-scope token (default `write:` on every
|
||||||
|
category, including admin for site admins) is a click on our consent page, which shows an
|
||||||
|
attacker-chosen name (for example "Claude") and the redirect URI. The design is documented in
|
||||||
|
ADR 0004; this is an owner decision on residual risk.
|
||||||
|
- Fix (owner to choose): add `OAUTH_REDIRECT_HOSTS`, an allowlist of redirect hosts accepted at
|
||||||
|
registration (default: loopback plus the hosted clients the owner uses, for example
|
||||||
|
`claude.ai` and `chat.mistral.ai`), with a clear registration error otherwise; on the consent
|
||||||
|
page show the redirect host first and prominently and the client's registration age. Or accept
|
||||||
|
the risk and record it as a waiver in `docs/security.md`.
|
||||||
|
- Negative test (`tests/security/test_oauth_redirects.py`): with the allowlist set,
|
||||||
|
registering `https://evil.example/cb` returns `invalid_redirect_uri`; allowlisted and loopback
|
||||||
|
URIs register; existing rules (http on non-loopback, fragments, credentials, custom schemes)
|
||||||
|
stay rejected.
|
||||||
|
|
||||||
|
### SEC-06 (medium): fork pull requests run Docker-capable jobs on the shared runner
|
||||||
|
|
||||||
|
- Where: `.gitea/workflows/ci.yml:22-25` (`pull_request` to `dev` and `main`), jobs
|
||||||
|
`test-integration` (`ci.yml:119-144`) and `test-mcp` (`ci.yml:153-185`).
|
||||||
|
- Description: a pull request from a fork runs its own workflow and test code, and these jobs
|
||||||
|
use the runner's Docker daemon (building images, starting containers, joining networks).
|
||||||
|
Access to the daemon is root-equivalent on the host that other tenants share. Secrets are not
|
||||||
|
passed to fork runs, but the host is.
|
||||||
|
- Fix: enable "require approval for workflows from forks" (or the equivalent runner setting) in
|
||||||
|
Gitea, and guard the Docker jobs with
|
||||||
|
`if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository`.
|
||||||
|
Record the setting in `docs/deployment.md` or the CI notes.
|
||||||
|
- Negative test (`tests/security/test_ci_workflows.py`): parse the workflow YAML; every job that
|
||||||
|
calls `docker` (or the integration and MCP smoke jobs by name) carries the fork guard.
|
||||||
|
|
||||||
|
### SEC-07 (medium): CI actions pinned by mutable tags, one of them receives the registry token
|
||||||
|
|
||||||
|
- Where: `.gitea/workflows/ci.yml:40, 63-69, 90-108, 124-130, 160-166, 206, 242, 299-305,
|
||||||
|
324-330`; `.gitea/workflows/publish.yml:22-28, 47-53, 77-85, 138`. `docker/login-action@v3`
|
||||||
|
receives `REGISTRY_TOKEN` (`ci.yml:242-248`).
|
||||||
|
- Description: `actions/checkout@v4`, `actions/setup-python@v5`, `astral-sh/setup-uv@v5`,
|
||||||
|
`actions/upload-artifact@v4` and `docker/login-action@v3` resolve to whatever the tag points to
|
||||||
|
at run time. A compromised or retagged action runs with the job's secrets and the Docker
|
||||||
|
daemon. `actions/checkout` also persists the job token in `.git/config` for later steps.
|
||||||
|
- Fix: pin every `uses:` to a full commit SHA with the version in a comment; replace
|
||||||
|
`docker/login-action` with `echo "$REGISTRY_TOKEN" | docker login --username ... --password-stdin`;
|
||||||
|
set `persist-credentials: false` on checkout where pushing is not needed.
|
||||||
|
- Negative test (`tests/security/test_ci_workflows.py`): every `uses:` value in
|
||||||
|
`.gitea/workflows/*.yml` matches `@[0-9a-f]{40}$`; no step passes `secrets.*` to a third-party
|
||||||
|
action's `with:`.
|
||||||
|
|
||||||
|
### SEC-08 (low): rate limiting gaps
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/components.py:37` (`RATE_LIMITED_PATHS` lacks `/token` and
|
||||||
|
`/revoke`), `src/aegis_gitea_mcp/rate_limit.py:142-163` (per-IP key is the full address),
|
||||||
|
`src/aegis_gitea_mcp/transports.py:168-188`, `transports.py:400-419`.
|
||||||
|
- Description: `/token` and `/revoke` (client secret checks, store writes) are not limited. Each
|
||||||
|
IPv6 address is its own bucket, so a client with a /64 is effectively unlimited. Behind a
|
||||||
|
proxy whose address is not in `FORWARDED_ALLOW_IPS` (the default `127.0.0.1` does not match a
|
||||||
|
Docker network), every request shares the proxy's bucket, and because the limit runs before
|
||||||
|
authentication, any unauthenticated client can exhaust the budget of all authenticated users.
|
||||||
|
- Fix: add `/token` and `/revoke` to `RATE_LIMITED_PATHS`; key IPv6 clients by their /64; log a
|
||||||
|
one-time warning when a request carries `X-Forwarded-For` from a peer outside
|
||||||
|
`FORWARDED_ALLOW_IPS`.
|
||||||
|
- Negative test (`tests/security/test_rate_limits.py`): limit+1 requests to `/token` get 429;
|
||||||
|
two addresses in one IPv6 /64 share a bucket; a forwarded header from an untrusted peer logs
|
||||||
|
the warning once.
|
||||||
|
|
||||||
|
### SEC-09 (low): OAuth pre-sign-in state can be flushed by unauthenticated floods
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/provider.py:139-159` (`_ExpiringMap.put` evicts the oldest
|
||||||
|
entry), `provider.py:80-85`, `provider.py:333-356` (client eviction),
|
||||||
|
`src/aegis_gitea_mcp/oauth/store.py:250-275` (synchronous fsync on the event loop).
|
||||||
|
- Description: 1024 `/authorize` requests (needs only a self-registered client) evict every
|
||||||
|
legitimate pending sign-in; 500 registrations evict recently registered clients that have
|
||||||
|
not finished signing in. Every registration writes and fsyncs the whole store on the event
|
||||||
|
loop. Impact: sign-in denial of service, no data exposure.
|
||||||
|
- Fix: per-IP quota on pending requests and registrations (for example 20 each); never evict a
|
||||||
|
client younger than `PENDING_TTL`; run `OAuthStore._write` in a worker thread
|
||||||
|
(`anyio.to_thread.run_sync`).
|
||||||
|
- Negative test (`tests/security/test_oauth_input_bounds.py`): 1100 `/authorize` calls from one
|
||||||
|
IP do not evict a pending request from another IP; a client registered a minute ago survives
|
||||||
|
a registration flood.
|
||||||
|
|
||||||
|
### SEC-10 (low): base URLs accept whitespace and control characters; plain http Gitea is silent
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/config.py:87-103` (`_normalize_http_url` validates the parsed URL
|
||||||
|
but returns the raw string), `config.py:180-201`.
|
||||||
|
- Description: `https://gi\ttea.example.com`, `https://gitea.example.com/a\nb` and values with
|
||||||
|
spaces pass validation; `urlsplit` strips tabs and newlines for the check while the raw value
|
||||||
|
is used later, so the validated and the used URL can differ. A plain `http://` `GITEA_URL` to a
|
||||||
|
non-loopback host is accepted at serve time without any warning, although the admin token, the
|
||||||
|
OAuth client secret and user tokens then travel in clear text (setup warns, `serve` does not).
|
||||||
|
- Fix: reject any whitespace or non-printable character in both URLs; return the URL rebuilt
|
||||||
|
from its parsed parts; print a startup warning (or refuse unless an explicit
|
||||||
|
`ALLOW_INSECURE_GITEA_URL=true`) for http to a non-loopback host.
|
||||||
|
- Negative test (`tests/security/test_base_url_validation.py`): parametrized rejection of tab,
|
||||||
|
newline, carriage return, space, NUL and bidi controls in `GITEA_URL` and `PUBLIC_BASE_URL`;
|
||||||
|
existing rejections (scheme, credentials, query, fragment, bad port, missing host, http public
|
||||||
|
base URL) kept; `serve` with `GITEA_URL=http://gitea.internal` prints the warning.
|
||||||
|
|
||||||
|
### SEC-11 (low): audit log created with umask defaults; unkeyed hash chain
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/audit.py:36-39`, `audit.py:64-69`.
|
||||||
|
- Description: `DATA_DIR` is created with the default mode (0755 under a 022 umask) and the
|
||||||
|
audit log with 0644; only the OAuth store later tightens the directory, so in admin-token mode
|
||||||
|
other local users can read logins, paths and timing of every call. The audit path is opened
|
||||||
|
without `O_NOFOLLOW`. The SHA-256 chain has no key, so anyone with write access can rewrite
|
||||||
|
it consistently, and truncating the tail is not detectable.
|
||||||
|
- Fix: create the directory with 0700 and open the log with
|
||||||
|
`os.open(path, O_WRONLY | O_APPEND | O_CREAT | O_NOFOLLOW, 0o600)`; document off-host
|
||||||
|
shipping as the integrity control, optionally add an `AUDIT_HMAC_KEY` for a keyed chain.
|
||||||
|
- Negative test (`tests/security/test_audit_file.py`): after the first call the log is 0600 and a
|
||||||
|
newly created `DATA_DIR` is 0700; a symlink at the audit path makes startup exit 2.
|
||||||
|
|
||||||
|
### SEC-12 (low): tool audit entries do not record the client, session or source IP
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/server.py:509-537`.
|
||||||
|
- Description: in OAuth mode an entry names the Gitea login but not which MCP client or grant
|
||||||
|
made the call, nor the source address, which limits incident response (for example finding
|
||||||
|
which registered client a stolen token belonged to).
|
||||||
|
- Fix: add `client_id` from the verified access token, a short hash of the session id and the
|
||||||
|
client IP (when on HTTP) to the tool invocation entry.
|
||||||
|
- Negative test (`tests/security/test_audit_fail_closed.py` or a sibling): an OAuth call's entry
|
||||||
|
contains the client id and no token or session id in clear.
|
||||||
|
|
||||||
|
### SEC-13 (low): token store directory ownership is not verified
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/store.py:133-143`.
|
||||||
|
- Description: an existing `DATA_DIR` owned by another user, or one that stays group or world
|
||||||
|
writable after the chmod attempt, only produces a warning. Whoever controls the directory can
|
||||||
|
replace the store with an older copy (resurrecting revoked grants) or delete it.
|
||||||
|
- Fix: refuse (raise `OAuthStoreError`) when the directory's owner is not the effective user or
|
||||||
|
it is still writable by group or others.
|
||||||
|
- Negative test (`tests/security/test_token_store.py`): with a directory reported as owned by
|
||||||
|
another uid (monkeypatched `os.stat`) or mode 0777 that cannot be tightened, loading or saving
|
||||||
|
fails with a clear message.
|
||||||
|
|
||||||
|
### SEC-14 (low): CI cleanup and credential handling on the shared Docker daemon
|
||||||
|
|
||||||
|
- Where: `scripts/ci/cleanup_job_resources.sh:43-48`, `.gitea/workflows/ci.yml:281-290`,
|
||||||
|
`.gitea/workflows/publish.yml:150-157`.
|
||||||
|
- Description: the cleanup removes every container whose `ancestor` is one of the given refs,
|
||||||
|
and the build job passes the shared tags `:latest`, `:main` and `:dev`; on a daemon that also
|
||||||
|
runs a deployment of this image that can remove containers that are not the job's.
|
||||||
|
`docker logout` acts on the shared credential store when the job is not containerized.
|
||||||
|
`uv publish --password "${REGISTRY_TOKEN}"` puts the token on the command line.
|
||||||
|
- Fix: remove containers only for per-run refs (`aegis-ci-<run id>-*` and the job's own
|
||||||
|
`:sha-` tag); for shared tags only untag (`docker image rm` of the tag); pass the publish
|
||||||
|
token through `UV_PUBLISH_PASSWORD`.
|
||||||
|
- Negative test (`tests/security/test_ci_workflows.py`): run the cleanup script with a fake
|
||||||
|
`docker` on `PATH` that records argv; `rm -f` is only issued for containers of per-run refs;
|
||||||
|
`publish.yml` contains no `--password` argument.
|
||||||
|
|
||||||
|
### SEC-15 (low): `tests/security` missing; publish not gated on the security job
|
||||||
|
|
||||||
|
- Where: `.gitea/workflows/ci.yml:337`, `.gitea/workflows/publish.yml` (needs only lint and
|
||||||
|
test).
|
||||||
|
- Description: `pytest tests/security` exits with code 4 (path not found), so the CI `security`
|
||||||
|
job fails today. The publish workflow ships packages without gitleaks, pip-audit or the
|
||||||
|
security tests.
|
||||||
|
- Fix: S2 creates the suite listed below; add a `security` job (pip-audit, ruff `S`, security
|
||||||
|
tests, gitleaks) to `publish.yml` and make `publish` need it.
|
||||||
|
- Negative test: the suite itself; a CI workflow test asserting `publish` needs the security
|
||||||
|
job.
|
||||||
|
|
||||||
|
### SEC-16 (low): no container image vulnerability scan
|
||||||
|
|
||||||
|
- Where: `docker/Dockerfile:6-7`, `.gitea/workflows/ci.yml` (`build-and-push`).
|
||||||
|
- Description: base image digests are pinned and refreshed by hand; OS package advisories in
|
||||||
|
the image are never checked.
|
||||||
|
- Fix: add a scan step (Trivy or Grype pinned by digest) on the built image, failing on high and
|
||||||
|
critical with a waiver file, and a scheduled digest refresh reminder.
|
||||||
|
- Negative test: CI only (the step runs and fails on a known vulnerable test image).
|
||||||
|
|
||||||
|
### SEC-17 (low): consent POST accepts a missing `Origin` header
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/callback.py:138-140`; ADR 0004 says approval needs "a
|
||||||
|
matching `Origin`".
|
||||||
|
- Description: the `SameSite=Strict` double-submit cookie is the effective CSRF control and is
|
||||||
|
sound, but the code and the ADR disagree. Browsers send `Origin` on form POSTs.
|
||||||
|
- Fix: require `Origin` to equal the public origin (fall back to `Referer` when `Origin` is
|
||||||
|
absent), or correct the ADR.
|
||||||
|
- Negative test (`tests/security/test_oauth_csrf.py`): POST without `Origin` and `Referer`,
|
||||||
|
with `Origin: null`, and with a foreign origin all get 400 and no redirect.
|
||||||
|
|
||||||
|
### SEC-18 (low): unvalidated argument names and unknown tool names reach the audit log
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/server.py:412-413` (argument names recorded before validation),
|
||||||
|
`server.py:428-431` (unknown tool name echoed and audited).
|
||||||
|
- Description: validation messages carefully avoid caller-chosen argument names because "they
|
||||||
|
could carry a secret", but the audit entry records them before validation, and an unknown tool
|
||||||
|
name (up to 128 characters) is written to the audit log and echoed in the error. Pattern
|
||||||
|
masking still runs, so only non-pattern secrets are affected.
|
||||||
|
- Fix: record only argument names that are properties of the tool's schema (plus a count of
|
||||||
|
rejected ones); record unknown tool names as a fixed marker plus length, and do not echo them.
|
||||||
|
- Negative test (`tests/security/test_redaction.py`): a call with an extra argument named like a
|
||||||
|
40-hex token, and a call to a tool named like one, leave neither value in the audit file or
|
||||||
|
the error.
|
||||||
|
|
||||||
|
### SEC-19 (low): one fixed cookie name for concurrent sign-ins in the same browser
|
||||||
|
|
||||||
|
- Where: `src/aegis_gitea_mcp/oauth/callback.py:56-60`, `callback.py:150-155`.
|
||||||
|
- Description: a second sign-in started in the same browser overwrites the first flow's binding
|
||||||
|
cookie. The first callback then fails and writes a high-severity
|
||||||
|
`oauth_callback_binding_mismatch` audit event, a false alarm that trains operators to ignore
|
||||||
|
the one event that signals a confused-deputy attempt.
|
||||||
|
- Fix: suffix the binding cookie name with a short hash of the Gitea `state`
|
||||||
|
(`__Host-aegis-oauth-flow-<8 hex>`), and look it up by the state on the callback.
|
||||||
|
- Negative test (`tests/security/test_oauth_csrf.py`): two interleaved flows in one browser
|
||||||
|
both complete; a callback without its own cookie still fails and is audited.
|
||||||
|
|
||||||
|
## Negative tests S2 must add to `tests/security/`
|
||||||
|
|
||||||
|
Some of these duplicate existing unit tests on purpose: the security suite is the one CI and
|
||||||
|
reviewers read, and it must stay green independently of refactors elsewhere. Reuse the helpers
|
||||||
|
in `tests/test_oauth_helpers.py` and `tests/test_executor_helpers.py`.
|
||||||
|
|
||||||
|
1. `test_auth_required.py`
|
||||||
|
- `/mcp` without `Authorization` gets 401 with `WWW-Authenticate` (bearer-only app and OAuth
|
||||||
|
app).
|
||||||
|
- Wrong bearer gets 401: same length, shorter, longer, empty, wrong scheme (`Basic`, `token`),
|
||||||
|
trailing whitespace, the bearer of another deployment.
|
||||||
|
- The static bearer is compared with `hmac.compare_digest` on digests (spy).
|
||||||
|
- OAuth: expired, revoked, other-resource and unknown `agm_` tokens get 401; an `agm_` token
|
||||||
|
never resolves to the admin identity (Gitea mock sees the user's token only).
|
||||||
|
- Route inventory: the set of routes reachable without authentication is exactly
|
||||||
|
`/`, `/health`, the `.well-known` documents and the OAuth routes; adding a route fails the
|
||||||
|
test.
|
||||||
|
2. `test_startup_fail_closed.py`
|
||||||
|
- Non-loopback binds (`0.0.0.0`, `::`, a public IP, a host name, `127.1`, `0`) without bearer
|
||||||
|
or OAuth exit 2 and never call `uvicorn.run`.
|
||||||
|
- Half-configured OAuth, a bearer shorter than 32 characters, an http `PUBLIC_BASE_URL` on a
|
||||||
|
public host, an unwritable audit log and (SEC-11) a symlinked audit path all exit 2.
|
||||||
|
3. `test_redaction.py`
|
||||||
|
- With all secrets configured (`GITEA_TOKEN`, `MCP_BEARER_TOKEN`, `GITEA_OAUTH_CLIENT_SECRET`,
|
||||||
|
`TOKEN_ENCRYPTION_KEY`, a user's Gitea access and refresh tokens, an issued `agm_` token),
|
||||||
|
drive a Gitea error that echoes each of them and a success body that contains each of them:
|
||||||
|
none appears in the result, the error message, the audit file, or captured logs at DEBUG.
|
||||||
|
- `uvicorn.access` records for `/oauth/callback?code=...&state=...` and
|
||||||
|
`/oauth/consent?request=...` contain `[redacted]` instead of the query.
|
||||||
|
- `repr` of `Settings`, `Identity`, `SessionRecord`, `UpstreamTokens` and
|
||||||
|
`PendingAuthorization` contains no secret.
|
||||||
|
- Validation errors never contain argument values (SEC-18 for names).
|
||||||
|
4. `test_secret_echo.py`
|
||||||
|
- `gitea_actions_repo_list_secrets` and `gitea_actions_org_list_secrets` return the names
|
||||||
|
Gitea sends; the update tools never echo the submitted `data`, including in a Gitea 422
|
||||||
|
error that repeats it.
|
||||||
|
- SEC-02: hook list, get and edit responses never contain `authorization_header` values, in
|
||||||
|
every `SECRET_DETECTION_MODE`.
|
||||||
|
- Submitted `password`, `secret`, `token`, `authorization_header` and migration credentials
|
||||||
|
are redacted from results.
|
||||||
|
- Credential-issuing operations return the new credential (documented exemption, W-07) but
|
||||||
|
the audit file and logs do not contain it.
|
||||||
|
5. `test_malformed_input.py`
|
||||||
|
- Unknown arguments, wrong types, missing required arguments rejected before Gitea is
|
||||||
|
contacted.
|
||||||
|
- Path values with `..`, `.`, empty segment, `\`, `?`, `#`, `%`, `%2e`, NUL, CR, LF, U+2028
|
||||||
|
rejected for catalog path parameters and for the raw tool path; a raw path starting `//`
|
||||||
|
rejected.
|
||||||
|
- Query names `token`, `access_token`, `sudo`, `_sudo` in any case rejected.
|
||||||
|
- `sudo` with CR/LF, spaces or over 255 characters rejected.
|
||||||
|
- Upload: oversized base64 rejected before decoding, invalid base64, filenames with quotes or
|
||||||
|
CR/LF, unknown file keys.
|
||||||
|
- Raw method outside the enum; `READ_ONLY` with a non-GET raw call.
|
||||||
|
- Deeply nested JSON arguments and bodies give an error result, never an exception.
|
||||||
|
- HTTP: body over the cap gets 413, non-JSON content type 400, foreign `Host` 421, foreign
|
||||||
|
`Origin` 403.
|
||||||
|
6. `test_base_url_validation.py`
|
||||||
|
- `GITEA_URL` and `PUBLIC_BASE_URL`: non-http schemes, credentials, query, fragment, bad port,
|
||||||
|
missing host, whitespace and control characters (SEC-10), http public base URL on a public
|
||||||
|
host; trailing `/api/v1` normalized.
|
||||||
|
- `MCP_PATH`, `FORWARDED_ALLOW_IPS` and `OAUTH_SCOPES` (unknown scope) rejections.
|
||||||
|
- Redirect URIs at registration: http on non-loopback, fragment, credentials, `javascript:`,
|
||||||
|
custom schemes rejected.
|
||||||
|
7. `test_oauth_csrf.py` (SEC-17, SEC-19): consent without the CSRF cookie, with a mismatched
|
||||||
|
token, without or with a foreign `Origin`; callback without the binding cookie, with a
|
||||||
|
replayed or expired state; none of them redirects.
|
||||||
|
8. `test_oauth_input_bounds.py` (SEC-01, SEC-09).
|
||||||
|
9. `test_audit_fail_closed.py` and `test_audit_file.py` (SEC-03, SEC-11, SEC-12).
|
||||||
|
10. `test_env_file.py` (SEC-04).
|
||||||
|
11. `test_token_store.py`: file mode 0600, directory 0700, symlink refused, wrong key stops
|
||||||
|
startup without wiping, store content is not plaintext, our tokens only as hashes, SEC-13.
|
||||||
|
12. `test_rate_limits.py` (SEC-08).
|
||||||
|
13. `test_ci_workflows.py` (SEC-06, SEC-07, SEC-14, SEC-15).
|
||||||
|
|
||||||
|
## B5 checker verdict
|
||||||
|
|
||||||
|
Verdict: CHANGES REQUIRED.
|
||||||
|
|
||||||
|
Blocking:
|
||||||
|
|
||||||
|
- SEC-01: the unauthenticated consent POST reads unbounded multipart bodies into memory and
|
||||||
|
tmpfs. This is B5 code and reproducible with one request. Fix and its negative test are
|
||||||
|
required before B5 is accepted.
|
||||||
|
|
||||||
|
Non-blocking (fix in S2, or record as waivers): SEC-05 (redirect allowlist, owner decision),
|
||||||
|
SEC-09, SEC-12, SEC-13, SEC-17 (code or ADR), SEC-19.
|
||||||
|
|
||||||
|
What was checked and holds:
|
||||||
|
|
||||||
|
- Identity separation: an access token with the OAuth marker always resolves to its own
|
||||||
|
session or fails; it never falls back to the admin token, even when the session ends or the
|
||||||
|
upstream refresh fails. Only the static bearer (or no token on stdio and unauthenticated
|
||||||
|
loopback) reaches admin-token mode; an OAuth-only deployment fails closed for it. The session
|
||||||
|
login is checked against the token subject.
|
||||||
|
- Authorization server: DCR client ids are minted by the SDK; PKCE S256 is mandatory towards
|
||||||
|
clients and used towards Gitea; redirect URIs are validated at registration and matched by the
|
||||||
|
SDK; codes are single use, bound to the client, replay revokes the grant; refresh tokens rotate
|
||||||
|
and reuse revokes the grant; revocation is limited to the owning client by the SDK; tokens are
|
||||||
|
bound to the MCP resource and scope.
|
||||||
|
- Browser flow: consent CSRF double submit with a `SameSite=Strict` `__Host-` cookie, a
|
||||||
|
browser-binding cookie on the callback, single-use states, no redirect on any invalid callback,
|
||||||
|
`no-store`, `no-referrer`, `DENY` framing and CSP on every page, HTML escaping of the client
|
||||||
|
name and redirect URI, no request parameter echoed.
|
||||||
|
- Upstream: confidential client with form-encoded credentials, no redirects followed, coarse
|
||||||
|
error kinds only, login validated against a strict pattern, token type and length checked,
|
||||||
|
unknown Gitea scopes rejected at configuration.
|
||||||
|
- Store: Fernet with an HKDF-SHA256 key, mode 0600 through `O_EXCL` temporary files and atomic
|
||||||
|
rename, symlinks refused, a wrong key or corrupt file stops startup without wiping, our tokens
|
||||||
|
stored only as hashes, pruning of expired tokens and abandoned sessions, bounded clients and
|
||||||
|
sessions per login.
|
||||||
|
- Secret handling: the user's refresh token, the store key and the client secret are redacted
|
||||||
|
from results; issued `agm_` tokens are masked by pattern; access-log query strings are dropped.
|
||||||
|
- Quality gates at `ce5ae6c`: 157 OAuth tests and the whole unit suite pass (846 passed, 23
|
||||||
|
skipped), coverage 98.2 percent, `ruff check`, `ruff format --check` and `mypy` clean. The docs
|
||||||
|
drift check failed only on the stale `docs/security.md`, replaced in this changeset.
|
||||||
+209
-82
@@ -1,108 +1,235 @@
|
|||||||
# Security
|
# Security
|
||||||
|
|
||||||
## Core Controls
|
AegisGitea-MCP is a full-access bridge into Gitea (ADR 0001): whoever can call it holds the
|
||||||
|
power of the Gitea token it uses, which for the admin token means the whole instance. The
|
||||||
|
server does not decide which operations are safe. It guarantees a small set of invariants
|
||||||
|
(authentication off loopback, identity separation, audit, secret redaction, bounded and
|
||||||
|
validated input and output, untrusted-content labelling) and leaves exposure to the owner
|
||||||
|
(`READ_ONLY`, `TOOLSETS`, `DISABLED_TOOLS`, token scopes). The operator checklist is
|
||||||
|
[hardening.md](hardening.md); open review findings are in
|
||||||
|
[security-findings.md](security-findings.md).
|
||||||
|
|
||||||
- OAuth2/OIDC bearer-token authentication for MCP tool execution.
|
## Threat model
|
||||||
- OIDC discovery + JWKS validation cache for JWT tokens.
|
|
||||||
- Userinfo validation fallback for opaque OAuth tokens.
|
|
||||||
- Scope enforcement:
|
|
||||||
- `read:repository` for read tools.
|
|
||||||
- `write:repository` for write tools.
|
|
||||||
- Policy engine checks before tool execution.
|
|
||||||
- Per-IP and per-token rate limiting.
|
|
||||||
- Strict schema validation (`extra=forbid`).
|
|
||||||
- Tamper-evident audit logging with hash chaining.
|
|
||||||
- Secret sanitization for logs and tool output.
|
|
||||||
- Production-safe error responses (no internal stack traces).
|
|
||||||
|
|
||||||
## Threat Model
|
### Assets
|
||||||
|
|
||||||
### Why shared bot tokens are dangerous
|
| Asset | Where it lives | Impact if lost |
|
||||||
|
|---|---|---|
|
||||||
|
| Admin Gitea token (`GITEA_TOKEN`) | env file or container env; process memory | Full control of the instance, including Sudo as any user |
|
||||||
|
| MCP bearer (`MCP_BEARER_TOKEN`) | env file; remote clients' configuration | Equivalent to the admin token over HTTP |
|
||||||
|
| OAuth client secret (`GITEA_OAUTH_CLIENT_SECRET`) | env file | Impersonating this server towards Gitea's OAuth endpoints |
|
||||||
|
| Per-user Gitea access and refresh tokens | `DATA_DIR/oauth-store.json` (encrypted), process memory | Acting as each signed-in user, for as long as Gitea accepts the refresh token |
|
||||||
|
| Our issued `agm_` access and refresh tokens | clients; only SHA-256 hashes in the store | Acting as one user through this server until expiry or revocation |
|
||||||
|
| Token store and its key (`TOKEN_ENCRYPTION_KEY`) | store in `DATA_DIR`, key in env | Store plus key discloses every user's Gitea tokens and every client secret |
|
||||||
|
| Action secret values, webhook auth headers, minted credentials | in transit through tool calls | Credentials of CI, webhooks and other systems |
|
||||||
|
| Audit log (`DATA_DIR/audit.log`) | `DATA_DIR` | Loss of accountability; integrity is the asset, not secrecy |
|
||||||
|
|
||||||
- A single leaked bot token can expose all repositories that bot can access.
|
### Actors
|
||||||
- Access is not naturally bounded per end user.
|
|
||||||
- Blast radius is large and cross-tenant.
|
|
||||||
|
|
||||||
### Why token-in-URL is insecure
|
- Owner: configures the server, holds the admin token. Trusted.
|
||||||
|
- OAuth users: Gitea accounts that sign in through a client. Trusted with their own Gitea
|
||||||
|
permissions only.
|
||||||
|
- Remote MCP clients, including hosted ones (claude.ai, Mistral Le Chat): hold a bearer or an
|
||||||
|
OAuth token and act through a model that reads untrusted content.
|
||||||
|
- Malicious Gitea content authors: anyone who can write an issue, comment, file, commit
|
||||||
|
message, release note or wiki page the model may read.
|
||||||
|
- Network attackers: anyone who can reach the HTTP port or sit on the path to it or to Gitea.
|
||||||
|
- Other tenants of the shared CI host: jobs and services on the same runner and Docker daemon.
|
||||||
|
|
||||||
- URLs can be captured by reverse proxy logs, browser history, referer headers, and monitoring pipelines.
|
### Trust boundaries
|
||||||
- Bearer tokens must be passed in `Authorization` headers only.
|
|
||||||
|
|
||||||
### Why per-user OAuth reduces lateral access
|
1. MCP client to HTTP endpoint (`transports.py`): unauthenticated network input until a bearer
|
||||||
|
or `agm_` token is verified (`auth.py`, `oauth/provider.py`).
|
||||||
|
2. Browser to OAuth routes (`oauth/callback.py`, SDK `/authorize`, `/token`, `/register`,
|
||||||
|
`/revoke`): unauthenticated, cross-site capable.
|
||||||
|
3. Verified token to identity (`oauth/resolver.py`, `auth.py`): decides which Gitea token a
|
||||||
|
call uses.
|
||||||
|
4. Server to Gitea (`gitea/executor.py`, `oauth/upstream.py`): Gitea is trusted for
|
||||||
|
authorization decisions, its content is not trusted.
|
||||||
|
5. Tool result to model: everything Gitea returns crosses into a model context.
|
||||||
|
6. Process to disk (`audit.py`, `oauth/store.py`, `config.py` reading `.env`).
|
||||||
|
7. Repository to CI runner (`.gitea/workflows/*.yml`, `scripts/ci/*`): code under test runs
|
||||||
|
with access to the runner's Docker daemon.
|
||||||
|
|
||||||
- Each MCP request executes with the signed-in user token.
|
### Threats and mitigations
|
||||||
- Gitea authorization stays source-of-truth for repository visibility.
|
|
||||||
- A compromised token is limited to that user�s permissions.
|
|
||||||
|
|
||||||
## Resource-type-aware authorization
|
Spoofing
|
||||||
|
|
||||||
The public server runs in *service-PAT mode*: a privileged bot token makes the
|
| Threat | Mitigation | Implemented in |
|
||||||
actual Gitea calls while the per-user OAuth identity decides what the user may
|
|---|---|---|
|
||||||
reach. Repository calls are gated by the user's collaborator permission on
|
| Calling the endpoint without credentials | Non-loopback bind refuses to start without a bearer or OAuth; SDK `RequireAuthMiddleware` answers 401 | `transports.py` (`http_startup_errors`, `build_http_app`) |
|
||||||
`owner/repo`. The rest of the Gitea surface — reachable through the
|
| Guessing the bearer | 32 character minimum; constant-time comparison of SHA-256 digests; per-IP and per-token rate limit before auth | `config.py`, `auth.py` (`StaticBearerVerifier`), `rate_limit.py` |
|
||||||
`gitea_request` escape hatch — is gated by **resource-type-aware authorization**
|
| Using a token minted for another resource | RFC 8707 resource binding and required scope `gitea` enforced by the SDK; `/authorize` with a foreign `resource` gets `invalid_target` | `transports.py` (`_auth_settings`), `oauth/provider.py` |
|
||||||
(`authz.py`). Every call is classified by `(method, path)` and enforced against
|
| Confused deputy through the shared Gitea OAuth app | Own consent page naming client and redirect URI, CSRF double submit with `SameSite=Strict` cookie, browser-binding cookie on the callback | `oauth/callback.py`, `oauth/provider.py` (residual: SEC-05) |
|
||||||
a type-specific rule. **Every decision fails closed**: a call that cannot be
|
| Code or refresh token replay | Single-use codes; replay or reuse of a rotated refresh token revokes the grant | `oauth/provider.py` |
|
||||||
classified, or whose permission cannot be positively verified against Gitea, is
|
|
||||||
denied and audited.
|
|
||||||
|
|
||||||
| Resource type | Rule (service-PAT mode) |
|
Tampering
|
||||||
|---------------|--------------------------|
|
|
||||||
| `repository` | Per-user collaborator permission on `owner/repo` (existing check). A repo path that cannot be parsed to `owner/repo` is denied. |
|
|
||||||
| `org` | The signed-in user must be a **verified member** of the target org (checked against Gitea, fail closed). |
|
|
||||||
| `user_owned` | A resource owned by a named user/org (`/users/{name}`, `/packages/{owner}`): allowed only when the owner is the caller, or the caller is a verified member of the owning org. |
|
|
||||||
| `user_self` | Token-owner-scoped endpoints (`/user`, `/notifications`): **denied** — in service-PAT mode the data belongs to the bot, not the caller. |
|
|
||||||
| `misc_global` | Instance-wide read-only utilities (markdown render, version, gitignore templates): reads allowed; writes denied. |
|
|
||||||
| `admin` | **Default deny.** Allowed only when the operator opts in (`RAW_API_ALLOW_SENSITIVE=true`) **and** the signed-in user is a verified Gitea site administrator. |
|
|
||||||
| `unknown` | Denied. |
|
|
||||||
|
|
||||||
This gate runs *in addition to* the policy engine and the `WRITE_MODE` gate — a
|
| Threat | Mitigation | Implemented in |
|
||||||
write call is denied unless write mode is on, policy allows it, and the
|
|---|---|---|
|
||||||
resource-type rule passes. In pure-OAuth mode (no service PAT) the user's own
|
| Path traversal or path reshaping into other Gitea routes | Path values rejected on `..`, `.`, empty segment, `\`, `?`, `#`, `%`, control characters; percent-encoded with `safe=""` except verified wildcard routes | `gitea/encoding.py` |
|
||||||
token already scopes every call at Gitea, so the extra gate is unnecessary.
|
| Header injection (Sudo, multipart) | Sudo restricted to `[A-Za-z0-9_.-]{1,255}`; filenames and content types single line, no quotes; header parameters of the spec never forwarded | `gitea/encoding.py`, `catalog/build.py` |
|
||||||
|
| Query smuggling of credentials or Sudo | `token`, `access_token`, `sudo`, `_sudo` refused as query names (any case) | `gitea/encoding.py` |
|
||||||
|
| Malformed arguments | JSON Schema validation (Draft 2020-12, `additionalProperties: false`) before anything runs; messages never echo values | `server.py` |
|
||||||
|
| Store tampering or wrong key | Fernet (authenticated encryption) with an HKDF-SHA256 key; undecryptable store stops startup instead of being wiped; symlinks refused | `oauth/store.py` (rollback residual: SEC-13) |
|
||||||
|
| Settings injection from a planted `.env` | none yet | `config.py` (SEC-04) |
|
||||||
|
|
||||||
Positive verification results (org membership, site-admin) are cached briefly
|
Repudiation
|
||||||
and bounded; only successful checks are cached, so a transient failure never
|
|
||||||
grants access.
|
|
||||||
|
|
||||||
## Full-API coverage: classified `gitea_request`
|
| Threat | Mitigation | Implemented in |
|
||||||
|
|---|---|---|
|
||||||
|
| Unrecorded tool calls | Exactly one hash-chained audit entry per call; a call is refused when the audit log cannot be opened; startup fails on an unwritable log | `server.py`, `audit.py`, `transports.py` (write failures after startup: SEC-03) |
|
||||||
|
| Undetected edits of the log | SHA-256 hash chain, `scripts/validate_audit_log.py` | `audit.py` (unkeyed chain: SEC-11) |
|
||||||
|
|
||||||
`gitea_request` exposes the long tail of the Gitea API that the curated typed
|
Information disclosure
|
||||||
tools do not cover, safely:
|
|
||||||
|
|
||||||
- **Deterministic read/write classifier.** `GET`/`HEAD` are reads; everything
|
| Threat | Mitigation | Implemented in |
|
||||||
else is a write. A small, explicit override table may only *downgrade*
|
|---|---|---|
|
||||||
provably side-effect-free render endpoints (markdown/markup) to reads — never
|
| Tokens in results or errors | Exact redaction of the identity token, refresh token, admin token, bearer, client secret, store key and submitted secret fields in every result and error; pattern masking per `SECRET_DETECTION_MODE` | `gitea/executor.py`, `gitea/redaction.py`, `security.py` |
|
||||||
the reverse — so a mutating call can never be misclassified as a read and slip
|
| Tokens in logs | JSON log formatter without tracebacks; query strings dropped from access logs; upstream errors reduced to coarse kinds | `logging_utils.py`, `transports.py` (`AccessLogQueryFilter`), `oauth/upstream.py` |
|
||||||
past the `WRITE_MODE` gate.
|
| Secret values echoed back | Submitted `password`, `secret`, `token` style fields and Action secret `data` redacted from the result | `gitea/encoding.py` (`sensitive_body_values`) (stored webhook auth headers on read: SEC-02) |
|
||||||
- **Known-path gate.** A request whose top path segment is not a recognized
|
| Leaks across identities | No cookie jar, no redirects followed, a fresh upstream client per OAuth exchange | `gitea/executor.py`, `oauth/upstream.py` |
|
||||||
Gitea `/api/v1` route prefix is denied (fail closed): unknown paths are never
|
| Stack traces | `EXPOSE_ERROR_DETAILS=false` by default; even when on, only the exception class name | `server.py`, `gitea/executor.py` |
|
||||||
passed straight through.
|
|
||||||
- **Admin/credential denylist.** `/admin`, `*tokens*`, `*secrets*`, `*hooks*`,
|
|
||||||
`*keys*`, `applications/oauth2`, and runner registration tokens are blocked for
|
|
||||||
every method (including `GET`) and cannot be re-opened from `policy.yaml` —
|
|
||||||
only `RAW_API_ALLOW_SENSITIVE=true` overrides them, and admin then still
|
|
||||||
requires a verified site administrator (see above).
|
|
||||||
|
|
||||||
## Prompt Injection Hardening
|
Denial of service
|
||||||
|
|
||||||
Repository content is treated as untrusted data.
|
| Threat | Mitigation | Implemented in |
|
||||||
|
|---|---|---|
|
||||||
|
| Large request bodies | SDK body limit on the MCP endpoint and SDK OAuth routes; upload cap checked before decoding | `server.py` (`max_request_body_size`), `gitea/encoding.py` (consent route: SEC-01) |
|
||||||
|
| Large or hostile responses | 16 MiB text cap, `MAX_BINARY_BYTES`, 64 KiB error bodies, JSON depth 100, item and character limits | `gitea/executor.py` |
|
||||||
|
| Request floods | Sliding window per IP and per token hash, bounded key sets, rate-limit events audited once per window | `rate_limit.py`, `transports.py` (gaps: SEC-08) |
|
||||||
|
| Unbounded OAuth state | Caps on clients (500), pending sign-ins (1024), codes (1024), sessions per login (50); store pruning | `oauth/provider.py`, `oauth/store.py` (flush by floods: SEC-09) |
|
||||||
|
|
||||||
- Tool outputs are bounded and sanitized.
|
Elevation of privilege
|
||||||
- No instructions from repository text are executed.
|
|
||||||
- Text fields are size-limited before returning to LLM clients.
|
|
||||||
|
|
||||||
## Secret Detection
|
| Threat | Mitigation | Implemented in |
|
||||||
|
|---|---|---|
|
||||||
|
| OAuth user reaching the admin token | Tokens with the OAuth marker only ever resolve to their session; only the static bearer (or no token on stdio and unauthenticated loopback) reaches admin-token mode; OAuth-only deployments fail closed | `oauth/resolver.py`, `auth.py` |
|
||||||
|
| OAuth user using Sudo | Sudo is sent with the caller's own token; Gitea honours it only for site admins | `gitea/executor.py` |
|
||||||
|
| Unknown Gitea scopes granting full access | Only granular `read:`/`write:` scopes accepted for `OAUTH_SCOPES` | `config.py` |
|
||||||
|
| Model tricked by injected content | Untrusted-content labelling and annotations (see below); exposure narrowed by configuration | `server.py`, `gitea/executor.py` |
|
||||||
|
| Untrusted code on the shared CI host | Per-run resource names and targeted cleanup | `.gitea/workflows/ci.yml`, `scripts/ci/cleanup_job_resources.sh` (fork PRs: SEC-06) |
|
||||||
|
|
||||||
Detected classes include:
|
### Residual risks and owner decisions
|
||||||
|
|
||||||
- API key and token patterns.
|
- Full access by default (ADR 0001). A leaked bearer equals the admin token. Sudo, admin,
|
||||||
- JWT-like tokens.
|
secrets and delete tools are enabled unless the owner narrows them.
|
||||||
- Private key block markers.
|
- Unauthenticated loopback mode is allowed for development. Any local user or process on the
|
||||||
- Common provider credential formats.
|
same host then acts with the admin token. Set a bearer on shared hosts.
|
||||||
|
- Hosted clients (claude.ai, Le Chat) hold the bearer or an OAuth refresh token outside the
|
||||||
|
owner's control. Prefer per-user OAuth and narrow `OAUTH_SCOPES`; the default grants
|
||||||
|
`write:` on every category, including admin for site admins.
|
||||||
|
- Credential-issuing operations (`gitea_user_create_token`, runner registration tokens such as
|
||||||
|
`gitea_admin_create_runner_registration_token`, OAuth2 application secrets,
|
||||||
|
`gitea_repo_create_deploy_token`) return the new credential to the model by design; only the
|
||||||
|
caller's own tokens are still redacted.
|
||||||
|
- Secret pattern masking is heuristic. Content containing third-party credentials (for example a
|
||||||
|
Gitea token committed to a file) is returned unless it matches a pattern.
|
||||||
|
- SSRF through Gitea features (migrations, webhooks, mirrors) is Gitea's responsibility; this
|
||||||
|
server only ever contacts `GITEA_URL`.
|
||||||
|
- The consent page is the only barrier against phishing with a self-registered client
|
||||||
|
(SEC-05).
|
||||||
|
- Key rotation for `TOKEN_ENCRYPTION_KEY` is not supported: a new key means moving the store
|
||||||
|
away and signing every user out.
|
||||||
|
- The audit chain is tamper-evident, not tamper-proof: anyone with write access to the file can
|
||||||
|
rewrite it consistently. Ship it off the host.
|
||||||
|
|
||||||
Behavior:
|
## Prompt injection
|
||||||
|
|
||||||
- `SECRET_DETECTION_MODE=mask`: redact in place.
|
Everything returned from Gitea is untrusted data. Every success envelope carries the fixed
|
||||||
- `SECRET_DETECTION_MODE=block`: replace secret-bearing values.
|
`untrusted_content` notice, the server instructions tell the model never to follow
|
||||||
- `SECRET_DETECTION_MODE=off`: disable sanitization (not recommended).
|
instructions found in Gitea content, and the server itself never interprets content (it does
|
||||||
|
not follow links, render, execute or chain calls). Tool annotations mark read-only and
|
||||||
|
destructive operations so clients can require confirmation. The realistic attack is a model
|
||||||
|
that reads a hostile issue and then calls a destructive tool; the defences are client-side
|
||||||
|
confirmation, `READ_ONLY`, `TOOLSETS`, `DISABLED_TOOLS` and narrow token scopes
|
||||||
|
([hardening.md](hardening.md)).
|
||||||
|
|
||||||
|
## Secret hygiene guarantees
|
||||||
|
|
||||||
|
- Settings hold secrets as `SecretStr`; `Identity`, `SessionRecord`, `UpstreamTokens` and the
|
||||||
|
pending authorization exclude tokens from `repr`.
|
||||||
|
- Results and errors: exact redaction of every known secret (identity token and refresh token,
|
||||||
|
`GITEA_TOKEN`, `MCP_BEARER_TOKEN`, `GITEA_OAUTH_CLIENT_SECRET`, `TOKEN_ENCRYPTION_KEY`,
|
||||||
|
secret fields submitted in the request), then pattern masking or blocking.
|
||||||
|
- Validation errors and argument errors never contain argument values.
|
||||||
|
- Audit entries carry argument names, never values, and are masked again before writing.
|
||||||
|
- Logs: JSON lines, exception type only, access-log query strings replaced by `[redacted]`.
|
||||||
|
- Our own tokens are stored only as SHA-256 hashes; the store file is encrypted, mode 0600,
|
||||||
|
written through `O_EXCL` temporary files and an atomic rename.
|
||||||
|
- Action secrets: Gitea's list endpoints (`gitea_actions_repo_list_secrets`,
|
||||||
|
`gitea_actions_org_list_secrets`) return names and descriptions only, and a submitted value
|
||||||
|
(`gitea_actions_update_repo_secret` and siblings) is redacted from the result.
|
||||||
|
- Known gap: a webhook's stored `authorization_header` is returned by hook read tools such as
|
||||||
|
`gitea_hook_repo_list_hooks` (SEC-02).
|
||||||
|
|
||||||
|
## Transport guarantees
|
||||||
|
|
||||||
|
- Fail-closed bind: `MCP_HOST` defaults to `127.0.0.1`; any non-loopback bind without a bearer
|
||||||
|
or a working OAuth setup exits with code 2. A half-configured OAuth setup also exits 2.
|
||||||
|
- Constant-time bearer check on SHA-256 digests, so neither content nor length leaks.
|
||||||
|
- DNS-rebinding protection on the MCP endpoint: `Host` must be a loopback name, the bind
|
||||||
|
address, the `PUBLIC_BASE_URL` host or an `ALLOWED_HOSTS` entry (421 otherwise); a foreign
|
||||||
|
`Origin` gets 403; POST must be `application/json`.
|
||||||
|
- Rate limits: sliding window per client IP and per Authorization hash on the MCP path and the
|
||||||
|
unauthenticated OAuth paths, before authentication. Client IPs come from `X-Forwarded-For`
|
||||||
|
only for `FORWARDED_ALLOW_IPS`.
|
||||||
|
- `PUBLIC_BASE_URL` must be https unless loopback. Gitea redirects are never followed.
|
||||||
|
- stdio: stdout carries JSON-RPC only; all logging goes to stderr.
|
||||||
|
|
||||||
|
### Per-user OAuth design
|
||||||
|
|
||||||
|
The server is an OAuth 2.1 authorization server for MCP clients (SDK provider, dynamic client
|
||||||
|
registration, PKCE S256 required) and a confidential PKCE client of Gitea. Flow: `/authorize`
|
||||||
|
parks the request and shows our consent page (client name, redirect URI, warning). Approval
|
||||||
|
needs the form CSRF token and the matching `SameSite=Strict` cookie and sets a
|
||||||
|
browser-binding cookie (`__Host-` prefixed and `Secure` on https, `HttpOnly`). The Gitea
|
||||||
|
callback is completed only in the browser holding that cookie; unknown, expired or replayed
|
||||||
|
states never redirect. Pages send `Cache-Control: no-store`, `Referrer-Policy: no-referrer`,
|
||||||
|
`X-Frame-Options: DENY` and a CSP with `frame-ancestors 'none'`. Redirect URIs must be https,
|
||||||
|
or http on loopback, without credentials or fragments. Issued tokens are opaque (`agm_` plus 256
|
||||||
|
random bits), hashed at rest, bound to the MCP resource; access tokens live 1 hour, refresh
|
||||||
|
tokens 30 days with rotation. Revocation ends the whole grant.
|
||||||
|
|
||||||
|
## Supply chain
|
||||||
|
|
||||||
|
- Base images pinned by digest (`python:3.12-slim`, `ghcr.io/astral-sh/uv`), non-root runtime
|
||||||
|
user, root-owned virtualenv, no build arguments carrying secrets, `.dockerignore` excludes
|
||||||
|
`.env`.
|
||||||
|
- Dependencies locked in `uv.lock` and installed with `uv sync --locked`.
|
||||||
|
- `pip-audit` over the exported lock (`scripts/ci/pip_audit.sh`, waivers in
|
||||||
|
`security/pip-audit-ignore.txt`).
|
||||||
|
- gitleaks pinned to 8.30.1 with sha256 verification, full-history scan with
|
||||||
|
`.gitleaks.toml`.
|
||||||
|
- ruff `S` (bandit) rules enabled in the normal lint and run separately in the `security` job.
|
||||||
|
- Gaps: CI actions are pinned by tag, not commit SHA (SEC-07); no container image scan
|
||||||
|
(SEC-16).
|
||||||
|
|
||||||
|
## Verification results
|
||||||
|
|
||||||
|
Run on 2026-10-14 against commit `ce5ae6c` (branch `feat/full-api-coverage`).
|
||||||
|
|
||||||
|
| Check | Command | Result |
|
||||||
|
|---|---|---|
|
||||||
|
| Security test suite | `uv run pytest tests/security --no-cov` | Pending: written in S2 (`tests/security/` does not exist yet, so the CI `security` job currently fails, SEC-15). Required tests are listed in [security-findings.md](security-findings.md). |
|
||||||
|
| Dependency audit | `bash scripts/ci/pip_audit.sh` (pip-audit 2.10.1, `--strict`) | No known vulnerabilities in 83 locked packages (runtime and dev groups). No ignores in use. |
|
||||||
|
| Bandit rules | `uv run ruff check --select S .` (ruff 0.17.0) | All checks passed, with the inline waivers listed below. |
|
||||||
|
| Secret scan | `scripts/ci/install_gitleaks.sh` into a scratch directory, then `gitleaks git --config .gitleaks.toml --redact --no-banner --verbose --exit-code 1 .` (gitleaks 8.30.1) | No leaks found. 131 commits in history (103 non-merge); gitleaks scanned 109 commits with content, about 6 MB. |
|
||||||
|
| Unit suite (context) | `uv run pytest` | 846 passed, 23 skipped (live Gitea and Docker tests), coverage 98.2 percent. |
|
||||||
|
|
||||||
|
## Waivers
|
||||||
|
|
||||||
|
Accepted findings, each with its reason. A new waiver needs a reason here in the same
|
||||||
|
changeset.
|
||||||
|
|
||||||
|
| Id | Finding | Where | Reason |
|
||||||
|
|---|---|---|---|
|
||||||
|
| W-01 | S104 (bind to all interfaces) | `transports.py` `_WILDCARD_HOSTS`, `tests/test_server_helpers.py`, `tests/test_config.py` | The addresses are compared or used as test input, never bound. The container binds 0.0.0.0 through `MCP_HOST`, which requires authentication. |
|
||||||
|
| W-02 | S105/S106 (hardcoded password) | `auth.py` (`token="static"`), `oauth/provider.py` (`TOKEN_PREFIX`, `token_type="Bearer"`), `audit.py` (mode string), `setup_snippets.py` (`TOKEN_PLACEHOLDER`) | Placeholders, public prefixes and type names, not secrets. |
|
||||||
|
| W-03 | S101 (assert) | `setup_cmd.py` lines 357 and 822 | Type narrowing after an explicit check; behaviour does not depend on the assert. |
|
||||||
|
| W-04 | S101, S105, S106, S108 in `tests/**`; S603, S607 in `tests/integration/harness.py` and `tests/mcp/helpers.py` | `pyproject.toml` per-file ignores, inline noqa | Test fixtures use fake tokens and temporary paths; the harness drives the trusted local Docker CLI with fixed argv. |
|
||||||
|
| W-05 | gitleaks allowlists | `.gitleaks.toml` | Only `generic-api-key` and `curl-auth-header`, only in `tests/`, `docs/`, removed legacy guides and one removed source file; every match was reviewed as a fake or placeholder. |
|
||||||
|
| W-06 | pip-audit | `security/pip-audit-ignore.txt` | Empty: no advisory is ignored. |
|
||||||
|
| W-07 | Credential-issuing operations bypass pattern masking | `gitea/executor.py` (`CREDENTIAL_ISSUING_OPERATIONS`) | Their purpose is to return a new credential; exact redaction of the caller's own tokens still applies. |
|
||||||
|
| W-08 | Unauthenticated `/health` and `/` report version and tool count, `/health` reports Gitea reachability | `transports.py` | Needed by container health checks and clients; reveals no configuration or secret. The Gitea probe is cached for 10 seconds. |
|
||||||
|
| W-09 | Pattern masking keeps the first and last 4 characters of a matched secret | `security.py` (`mask_secret`) | Applies only to pattern matches, which are at least 16 characters; the remainder keeps enough entropy, and the context helps users recognise the value. |
|
||||||
Reference in new issue
Block a user