Files
AegisGitea-MCP/docs/security-findings.md

36 KiB

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. 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.
  • 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.
  • 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.

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.