AOSSIE-Org / AOSSIE-Org/PictoPy

BUG: Prevent unauthenticated remote shutdown via /shutdown endpoint (require explicit enable + token protection)

オープン
#1,193 コメント 5 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
283
フォーク
679
平均マージ
7日 2時間
マージ済み PR(30日)
3

説明

### Is there an existing issue for this?

- [x] I have searched the existing issues

### What happened?

## Security: Prevent unauthenticated remote shutdown via `/shutdown` endpoint

### Description
The `/shutdown` backend endpoint currently allows any network caller to terminate the server process.

If the backend is reachable externally, this exposes a critical denial-of-service (DoS) vulnerability and administrative control risk.

Remote shutdown functionality should be **disabled by default** and only enabled explicitly with proper authentication safeguards.

---

### Severity

**Critical** — Remote administrative control exposure / Denial-of-Service

---

### Steps to Reproduce
1. Start the backend server.
2. Execute:

```bash
curl -X POST http://:/shutdown
```

3. The server terminates immediately (or schedules termination).
---

### Current Behavior

- Any unauthenticated caller can invoke `/shutdown`.
- No environment-based safeguard exists.
- No authentication mechanism is enforced.
- No confirmation or restriction is applied before process termination.

This means that if the backend is bound to `0.0.0.0` or deployed in a container/cloud environment without strict firewall rules, the shutdown endpoint becomes remotely exploitable.

---

### Security Impact

This exposes a critical Denial-of-Service (DoS) vector:

- Any attacker or accidental external request can immediately terminate the backend process.
- In production, this may cause service outages.
- In containerized deployments (Docker/Kubernetes), repeated shutdown calls could cause restart loops.
- There is no visibility or authentication barrier preventing misuse.

Even if currently intended for local development use, the endpoint poses risk if accidentally exposed.

---

### Expected Behavior

The shutdown mechanism should follow a **secure-by-default** principle:

1. Remote shutdown must be disabled unless explicitly enabled.
2. Enabling shutdown must require deliberate operator configuration.
3. If a shutdown token is configured, all shutdown requests must authenticate using that token.
4. Unauthorized or disabled attempts must return appropriate HTTP status codes.

#### Response behavior:

- `403 Forbidden` → Remote shutdown disabled.
- `401 Unauthorized` → Token required but missing/invalid.
- `200 OK` → Shutdown request accepted and scheduled.

---

### Proposed Solution (High-Level)

Introduce two environment-configurable settings:

- `ALLOW_REMOTE_SHUTDOWN` (default: `False`)
- `SHUTDOWN_TOKEN` (optional secret)

The `/shutdown` route should:

1. Immediately return `403` if remote shutdown is not explicitly enabled.
2. If a shutdown token is configured:
- Require `X-Shutdown-Token` header.
- Validate token.
- Return `401` if invalid.
3. Only proceed with delayed shutdown after validation passes.
4. Log blocked attempts without logging secret values.

This ensures backward compatibility while significantly improving safety.

---

### Acceptance Criteria

- When `ALLOW_REMOTE_SHUTDOWN=false` → `/shutdown` returns 403.
- When enabled and token configured:
- Missing or incorrect token → 401.
- Correct token → 200 and shutdown scheduled.
- No secret values logged.
- Tests added using FastAPI TestClient.
- README updated with secure usage documentation.

---

### Security Best Practices Consideration

- `SHUTDOWN_TOKEN` must be treated as a secret and never committed to the repository.
- Recommended to store via environment variables or secret manager.
- In future iterations, additional hardening could include:
- IP allowlisting.
- Admin authentication integration.
- Audit logging for shutdown attempts.

---

### Rationale

Administrative endpoints such as `/shutdown` should never be publicly callable without safeguards.

Applying secure-by-default principles prevents accidental exposure and aligns with standard backend security practices.

---

I would be happy to implement this enhancement and add associated unit tests if maintainers agree with this approach.

### Record

- [x] I agree to follow this project's Code of Conduct

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。