AegisGitea-MCP/docs/security-findings.md

# 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 | Status (S2) |
|---|---|---|---|
| SEC-01 | high | Unbounded request body on the unauthenticated consent POST | fixed |
| SEC-02 | medium | Stored webhook `authorization_header` returned by hook read tools | fixed |
| SEC-03 | medium | Audit write failures after startup do not stop later calls | fixed |
| SEC-04 | medium | Settings read a `.env` file from the process working directory | fixed |
| SEC-05 | medium | Any https redirect URI can be registered; the consent page is the only barrier | fixed, default waived (W-10) |
| SEC-06 | medium | Fork pull requests run Docker-capable jobs on the shared runner | fixed |
| SEC-07 | medium | CI actions pinned by mutable tags, one of them receives the registry token | fixed |
| SEC-08 | low | Rate limiting gaps (`/token`, `/revoke`, IPv6, proxy address) | fixed |
| SEC-09 | low | OAuth pre-sign-in state can be flushed by unauthenticated floods | fixed |
| SEC-10 | low | `GITEA_URL` and `PUBLIC_BASE_URL` accept whitespace and control characters; plain http Gitea is silent | fixed |
| SEC-11 | low | Audit log created with umask defaults; unkeyed hash chain | fixed, keyed chain waived (W-11) |
| SEC-12 | low | Tool audit entries do not record the client, session or source IP | fixed |
| SEC-13 | low | Token store directory ownership is not verified | fixed |
| SEC-14 | low | CI cleanup and credential handling on the shared Docker daemon | fixed |
| SEC-15 | low | `tests/security` missing, so the CI `security` job fails; publish is not gated on it | fixed |
| SEC-16 | low | No container image vulnerability scan | fixed |
| SEC-17 | low | Consent POST accepts a missing `Origin` header, unlike ADR 0004 says | fixed |
| SEC-18 | low | Unvalidated argument names and unknown tool names reach the audit log | fixed |
| SEC-19 | low | One fixed cookie name for concurrent sign-ins in the same browser | fixed |

All 19 findings were fixed or waived in S2 (status column and per-finding status lines below); the
negative tests live in `tests/security/`. The fixes found four further defects that are fixed in
the same changeset: secrets inside JSON object keys were not pattern masked (`fef80a5`), a
trailing newline passed the `sudo` pattern (`984673e`), a `data` field on an Action secret
response was not redacted (`421a650`), and configuration errors for `OAUTH_SCOPES` and
`TOOLSETS` echoed the offending values (`0f5f5cd`).

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

- Status: fixed in `d5ca530`: consent and callback routes wrapped in a 16 KiB body limit; 415 for any content type but `application/x-www-form-urlencoded` before a byte is read; form parsed with `max_files=0, max_fields=8, max_part_size=4096`. Tests: `tests/security/test_oauth_input_bounds.py`.
- 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

- Status: fixed in `5b622e6`: `SENSITIVE_RESPONSE_KEYS` (the request-side key list) redacts non-empty string values by key name at any depth in every result except credential-issuing operations, in all `SECRET_DETECTION_MODE` values. Tests: `tests/security/test_secret_echo.py`.
- 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

- Status: fixed in `43eebd3`: the write-ahead alternative: a `tool_call_started` entry is written before every known tool runs and the call is refused with `audit_unavailable` when it fails. Documented in `docs/audit-logging.md`. Tests: `tests/security/test_audit_fail_closed.py`.
- 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

- Status: fixed in `79aa4f7`: `./.env` loads only as a regular file owned by the user with mode 0600; `--env-file` and `AEGIS_ENV_FILE`; documented in README, configuration and troubleshooting. Tests: `tests/security/test_env_file.py`.
- 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

- Status: fixed, default waived (W-10) in `8be8592`: owner decision: optional `OAUTH_REDIRECT_ALLOWLIST` (exact URIs or `https://host/` prefixes, checked at registration and `/authorize`); the open default is waived with the consent page as mitigation, and the allowlist is recommended for public deployments. Tests: `tests/security/test_oauth_redirects.py`.
- 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

- Status: fixed in `766968f`: `test-integration`, `test-mcp` and `build-and-push` carry the fork guard. The Gitea setting "require approval for workflows from forks" is a manual step recorded in `docs/deployment.md`. Tests: `tests/security/test_ci_workflows.py`.
- 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

- Status: fixed in `766968f`: every `uses:` is pinned to a full commit SHA with a version comment; `docker/login-action` was replaced by `docker login --password-stdin` into a per-job `DOCKER_CONFIG`; `persist-credentials: false` on every checkout. Tests: `tests/security/test_ci_workflows.py`.
- 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

- Status: fixed in `f57c237`: `/token` and `/revoke` rate limited; IPv6 clients keyed by /64; `ForwardedHeadersMiddleware` applies forwarded headers for `FORWARDED_ALLOW_IPS` peers and logs one warning for any other peer that sends `X-Forwarded-For`. Tests: `tests/security/test_rate_limits.py`.
- 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

- Status: fixed in `87f048c`: per client address at most 20 pending sign-ins and 20 registrations per hour; clients younger than `PENDING_TTL` are never evicted; the store write runs in a worker thread. Tests: `tests/security/test_oauth_input_bounds.py`.
- 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

- Status: fixed in `d96c3ac`: whitespace, backslash and control characters rejected in both URLs, the URL is rebuilt from its parsed parts, `serve` warns for plain http to a non-loopback Gitea (a warning rather than a refusal, so existing lab setups keep working). Tests: `tests/security/test_base_url_validation.py`.
- 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

- Status: fixed, keyed chain waived (W-11) in `7307214`: directory 0700, log opened `O_NOFOLLOW` with mode 0600, an older loose log is tightened, startup exits 2 for a symlink. The optional `AUDIT_HMAC_KEY` was not added: off-host shipping is the documented integrity control. Tests: `tests/security/test_audit_file.py`.
- 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

- Status: fixed in `fd5e4b7`: tool entries carry `client_id`, a 12 character `session` hash and `client_ip`. Tests: `tests/security/test_audit_fail_closed.py`.
- 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

- Status: fixed in `6f7a3c0`: the store directory must be owned by the effective user and must not stay group or world writable; checked on load and on every save. Tests: `tests/security/test_token_store.py`.
- 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

- Status: fixed in `766968f`: cleanup removes containers only for per-run refs (`aegis-ci-*`, `:sha-*`) and only untags shared tags; the credential directory is per job; `uv publish` reads `UV_PUBLISH_PASSWORD`. Tests: `tests/security/test_ci_workflows.py` (fake `docker` on `PATH`).
- 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

- Status: fixed in `766968f`: `tests/security` exists (this suite) and `publish.yml` has a `security` job that `publish` needs (gitleaks, security tests, pip-audit, ruff `S`). Tests: `tests/security/test_ci_workflows.py`.
- 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

- Status: fixed in `766968f`: Trivy 0.76.0 pinned with sha256 (`scripts/ci/install_trivy.sh`) scans the built image in `build-and-push` before the push and fails on fixable HIGH or CRITICAL findings (`scripts/ci/scan_image.sh`, waivers in `security/trivyignore.txt`); `scan.yml` repeats it weekly as the digest refresh reminder. Verified locally against the built image (clean).
- 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

- Status: fixed in `782d416`: consent POST needs an `Origin` equal to the public origin, with the `Referer` origin as fallback; missing, `null` or foreign values are refused. Tests: `tests/security/test_oauth_csrf.py`.
- 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

- Status: fixed in `fd5e4b7`: only argument names that are schema properties are recorded (rest counted as `rejected_argument_count`); unknown tool names are recorded as `(unknown tool, N chars)` and not echoed. Tests: `tests/security/test_audit_fail_closed.py`.
- 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

- Status: fixed in `782d416`: the binding cookie is named per flow (`aegis-oauth-flow-<8 hex of the state hash>`). Tests: `tests/security/test_oauth_csrf.py`.
- 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 added to `tests/security/`

All of the files below exist now (the list is kept as the specification they were written
from). Results are in the "Verification results" section of [security.md](security.md).

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 (at `ce5ae6c`; the blocking item was fixed in S2, `d5ca530`, and
every non-blocking item below was fixed or waived, see the status column above).

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.