AOSSIE-Org / AOSSIE-Org/PictoPy

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

Đang mở
#1,193 5 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Python
Star
283
Fork
679
Merge trung bình
7 ngày 2 giờ
Pull request đã merge (30 ngày)
3

Mô tả

### 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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.