AOSSIE-Org / AOSSIE-Org/PictoPy

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

Aperta
#1,193 5 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Python
Stelle
283
Fork
679
Merge medio
7g 2h
PR unite (30g)
3

Descrizione

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

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.