diff --git a/FORK_CHANGES.md b/FORK_CHANGES.md index 69190136f..e85d15cbb 100644 --- a/FORK_CHANGES.md +++ b/FORK_CHANGES.md @@ -4,8 +4,8 @@ This file tracks **which changes are our own** (EmeraldHost-specific) versus ups [`pterodactyl/wings`](https://github.com/pterodactyl/wings). Use it during upgrades so our customizations are **not accidentally reverted** when pulling in upstream changes. -- **Baseline for this comparison:** upstream tag **`v1.13.1`** (`e771816`) -- **Last reviewed:** 2026-06-30 +- **Baseline for this comparison:** upstream tag **`v1.13.2`** (`28af6dd`) +- **Last reviewed:** 2026-08-03 - **Module path:** this fork is `github.com/Rene-Roscher/wings` (upstream is `github.com/pterodactyl/wings`). Version is injected at build time via ldflags (`-X .../system.Version=`); `system/const.go` stays `develop` and is **not** a divergence. @@ -23,6 +23,9 @@ our customizations are **not accidentally reverted** when pulling in upstream ch > heavily customized restore path. Upstream merges in `server/backup*`, `router/router_server_backup.go`, > `server/server.go` and `sftp/server.go` will almost always conflict — resolve by **keeping ours** > and grafting upstream's functional/security changes on top (that is exactly how v1.13.1 was merged). +> +> v1.13.2 was the exception: it only touched `router/tokens/**` plus three call sites and merged +> without a single conflict — see §4. --- @@ -72,6 +75,7 @@ our customizations are **not accidentally reverted** when pulling in upstream ch | `server/filesystem/archive.go` | Archiver no longer skips directory entries → **empty directories are preserved** in archives. `createCompressor()` refactor. | | `server/filesystem/archive_restore.go` | **Fork-new.** `DetectCompressionFormat` (magic bytes) + `CreateDecompressor`, wired into restore. | | `server/filesystem/compress_binary_test.go`, `server/backup/*_test.go`, `router/content_type_test.go`, `server/backup_*_test.go` | Fork-new regression suites for the above. Keep them passing. | +| `server/filesystem/archive_test.go` + `archive_stream_test.go` | **File layout divergence, not new behavior.** Our `archive_test.go` was replaced wholesale with tests for the fork-only `archive_restore.go`; upstream's `TestArchive_Stream` lives in `archive_stream_test.go` instead. Upstream edits to `archive_test.go` therefore land in the *wrong* file on merge — port them into `archive_stream_test.go` by hand. | **Router API (fork-only endpoints + customized handlers)** @@ -131,14 +135,24 @@ much as bugs/concerns in our own additions, worth fixing rather than defending o ## 4. NOT fork divergences — adopted from upstream (do **not** re-apply) -These show up around our customizations but are **upstream v1.13.1** code. Treating them as +These show up around our customizations but are **upstream** code. Treating them as fork changes risks duplicating or mis-merging them on the next upgrade. | Path | Reality | |------|---------| +| `router/tokens/websocket.go` → `isDenylisted()`, and `Denylisted()` on `FilePayload` / `BackupPayload` / `UploadPayload` (+ their new `user_uuid` claim) | **Upstream v1.13.2** (`28af6dd`, "update token validation"). Revocation checking was extracted out of `WebsocketPayload.Denylisted()` into a shared `isDenylisted()` and applied to the backup-download, file-download and file-upload one-time tokens, which previously only checked `IsUniqueRequest()`/scope. Also tightened `Before(t)` → `!After(t)`, so a token issued in the same second as the revocation is now denied. All four files are byte-identical to upstream — **keep them that way**. | +| `router/tokens/denylist_test.go` | **Upstream v1.13.2**, unmodified. Covers the four payload types above. Not a fork suite. | +| `router/router_download.go`, `router/router_server_files.go` → the `token.Denylisted() \|\|` guards | **Upstream v1.13.2** call sites. The surrounding files *are* fork-modified (module rename + activity logging), so these three one-liners are easy to lose in a conflict resolution — check they survive. | | `router/router_server_backup.go` SSRF cluster — `backupRestoreHttpClient`, `validateBackupDownloadUrl`, `parseBackupUuid`, `isBlockedBackupRestoreIP`, `isExplicitlyBlockedBackupRestoreIP`, `isAllowedBackupRestoreDestination`, `isSupportedBackupRestoreContentType`, `blockedBackupRestorePrefixes`, `backupDownloadError` | **Upstream v1.13.1** backup-restore SSRF hardening. The **only** fork edit in this cluster: the restore handler calls `backup.IsValidBackupContentType` instead of `isSupportedBackupRestoreContentType` (the latter is retained only for upstream parity + its test). | | `config/config.go` → `Backups.RestoreHostAllowlist` | **Upstream v1.13.1.** Pairs with the SSRF allowlist above. Not a fork field. | | `server/backup/backup.go` → `validateIdentifier()` / `normalizedIdentifier()` (+ `Path()` `path.Base` fallback) | **Upstream v1.13.1** UUID hardening. The fork uses them unchanged. | | `system/const.go` | Byte-identical to upstream (`Version = "develop"`). | -| `.github/FUNDING.yaml` (`github: [pterodactyl]`) | **Upstream default, unchanged** (`git diff e771816 HEAD` is empty). Stale for a fork (sponsorship points at upstream) but NOT our change — clean it up if desired, don't track it as a divergence. | +| `.github/FUNDING.yaml` (`github: [pterodactyl]`) | **Upstream default, unchanged** (`git diff 28af6dd HEAD` is empty). Stale for a fork (sponsorship points at upstream) but NOT our change — clean it up if desired, don't track it as a divergence. | | `.github/workflows/release.yaml` release-bot identity (`ci@pterodactyl.io` / `Pterodactyl CI`) | **Upstream default, unchanged.** Upstream's release.yaml already sets this identity. Not our divergence. | + +> ⚠️ **Panel coupling introduced by v1.13.2.** `isDenylisted()` **fails closed**: a token with no +> `iat`, no `server_uuid` or no `user_uuid` is rejected outright. The `user_uuid` claim is new in +> v1.13.2, so backup downloads, file downloads and file uploads only work against a Panel that +> puts `user_uuid` into those JWTs. Against an older Panel every such request returns +> `404 "The requested resource was not found on this server."` — deploy Panel **before** Wings, +> and if downloads/uploads start 404-ing after a Wings upgrade, this is the first thing to check. diff --git a/router/router_download.go b/router/router_download.go index c6e549431..2a6d2ab24 100644 --- a/router/router_download.go +++ b/router/router_download.go @@ -28,7 +28,7 @@ func getDownloadBackup(c *gin.Context) { } // Get the server using the UUID from the token. - if _, ok := manager.Get(token.ServerUuid); !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.BackupDownload) { + if _, ok := manager.Get(token.ServerUuid); !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.BackupDownload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) @@ -82,7 +82,7 @@ func getDownloadFile(c *gin.Context) { } s, ok := manager.Get(token.ServerUuid) - if !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.FileDownload) { + if !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.FileDownload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) diff --git a/router/router_server_files.go b/router/router_server_files.go index fd638c9e4..7a2936b90 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -634,7 +634,7 @@ func postServerUploadFiles(c *gin.Context) { } s, ok := manager.Get(token.ServerUuid) - if !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.FileUpload) { + if !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.FileUpload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) diff --git a/router/tokens/backup.go b/router/tokens/backup.go index c0f43e51a..269cf12fa 100644 --- a/router/tokens/backup.go +++ b/router/tokens/backup.go @@ -9,6 +9,7 @@ type BackupPayload struct { Scoped ServerUuid string `json:"server_uuid"` + UserUuid string `json:"user_uuid"` BackupUuid string `json:"backup_uuid"` UniqueId string `json:"unique_id"` } @@ -25,3 +26,9 @@ func (p *BackupPayload) GetPayload() *jwt.Payload { func (p *BackupPayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *BackupPayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} diff --git a/router/tokens/denylist_test.go b/router/tokens/denylist_test.go new file mode 100644 index 000000000..6f0be5171 --- /dev/null +++ b/router/tokens/denylist_test.go @@ -0,0 +1,173 @@ +package tokens + +import ( + "encoding/json" + "testing" + "time" + + "github.com/gbrlsnchs/jwt/v3" +) + +type denylistable interface { + Denylisted() bool +} + +func resetDenylistState(t *testing.T, bootTime time.Time) { + t.Helper() + + originalBootTime := wingsBootTime + wingsBootTime = bootTime + denylist.Clear() + userDenylist.Clear() + + t.Cleanup(func() { + wingsBootTime = originalBootTime + denylist.Clear() + userDenylist.Clear() + }) +} + +func payloadsIssuedAt(issuedAt time.Time, serverUUID, userUUID string) map[string]denylistable { + payload := func() jwt.Payload { + return jwt.Payload{ + IssuedAt: jwt.NumericDate(issuedAt), + JWTID: "token-id", + } + } + + return map[string]denylistable{ + "websocket": &WebsocketPayload{ + Payload: payload(), + ServerUUID: serverUUID, + UserUUID: userUUID, + }, + "file download": &FilePayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + "backup download": &BackupPayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + "file upload": &UploadPayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + } +} + +func TestUserBoundPayloadsHonorServerRevocation(t *testing.T) { + issuedAt := time.Now().Add(-time.Minute) + resetDenylistState(t, issuedAt.Add(-time.Minute)) + + payloads := payloadsIssuedAt(issuedAt, "server", "user") + for name, payload := range payloads { + if payload.Denylisted() { + t.Fatalf("expected fresh %s token to be accepted", name) + } + } + + DenyForServer("server", "user") + + for name, payload := range payloads { + if !payload.Denylisted() { + t.Fatalf("expected revoked %s token to be denied", name) + } + } +} + +func TestUserBoundPayloadsAllowTokensIssuedAfterRevocation(t *testing.T) { + revokedAt := time.Now() + resetDenylistState(t, revokedAt.Add(-time.Hour)) + userDenylist.Store("server:user", revokedAt) + + for name, payload := range payloadsIssuedAt(revokedAt, "server", "user") { + if !payload.Denylisted() { + t.Fatalf("expected %s token issued at revocation time to be denied", name) + } + } + + for name, payload := range payloadsIssuedAt(revokedAt.Add(time.Second), "server", "user") { + if payload.Denylisted() { + t.Fatalf("expected reissued %s token to be accepted", name) + } + } +} + +func TestDownloadPayloadsDecodeUserUUID(t *testing.T) { + const encoded = `{"server_uuid":"server","user_uuid":"user"}` + + var file FilePayload + if err := json.Unmarshal([]byte(encoded), &file); err != nil { + t.Fatal(err) + } + if file.UserUuid != "user" { + t.Fatalf("expected file payload user UUID to be decoded, got %q", file.UserUuid) + } + + var backup BackupPayload + if err := json.Unmarshal([]byte(encoded), &backup); err != nil { + t.Fatal(err) + } + if backup.UserUuid != "user" { + t.Fatalf("expected backup payload user UUID to be decoded, got %q", backup.UserUuid) + } +} + +func TestUserBoundPayloadsFailClosedWithoutRevocationClaims(t *testing.T) { + issuedAt := time.Now() + resetDenylistState(t, issuedAt.Add(-time.Hour)) + + tests := map[string]denylistable{ + "missing issued at": &FilePayload{ + ServerUuid: "server", + UserUuid: "user", + }, + "missing server UUID": &BackupPayload{ + Payload: jwt.Payload{IssuedAt: jwt.NumericDate(issuedAt)}, + UserUuid: "user", + }, + "missing user UUID": &UploadPayload{ + Payload: jwt.Payload{IssuedAt: jwt.NumericDate(issuedAt)}, + ServerUuid: "server", + }, + } + + for name, payload := range tests { + if !payload.Denylisted() { + t.Fatalf("expected token with %s to be denied", name) + } + } +} + +func TestUserBoundPayloadsRejectTokensIssuedBeforeBoot(t *testing.T) { + bootTime := time.Now() + resetDenylistState(t, bootTime) + + for name, payload := range payloadsIssuedAt(bootTime.Add(-time.Second), "server", "user") { + if !payload.Denylisted() { + t.Fatalf("expected pre-boot %s token to be denied", name) + } + } +} + +func TestUserRevocationIsScopedToServerAndUser(t *testing.T) { + issuedAt := time.Now().Add(-time.Minute) + resetDenylistState(t, issuedAt.Add(-time.Minute)) + DenyForServer("server", "user") + + for name, payload := range payloadsIssuedAt(issuedAt, "other-server", "user") { + if payload.Denylisted() { + t.Fatalf("expected %s token for another server to be accepted", name) + } + } + + for name, payload := range payloadsIssuedAt(issuedAt, "server", "other-user") { + if payload.Denylisted() { + t.Fatalf("expected %s token for another user to be accepted", name) + } + } +} diff --git a/router/tokens/file.go b/router/tokens/file.go index c60f39897..ffc19f779 100644 --- a/router/tokens/file.go +++ b/router/tokens/file.go @@ -10,6 +10,7 @@ type FilePayload struct { FilePath string `json:"file_path"` ServerUuid string `json:"server_uuid"` + UserUuid string `json:"user_uuid"` UniqueId string `json:"unique_id"` } @@ -25,3 +26,9 @@ func (p *FilePayload) GetPayload() *jwt.Payload { func (p *FilePayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *FilePayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} diff --git a/router/tokens/upload.go b/router/tokens/upload.go index ad5d16b0f..3c429935c 100644 --- a/router/tokens/upload.go +++ b/router/tokens/upload.go @@ -25,3 +25,9 @@ func (p *UploadPayload) GetPayload() *jwt.Payload { func (p *UploadPayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *UploadPayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} diff --git a/router/tokens/websocket.go b/router/tokens/websocket.go index 8ad3fb55e..07ad8f4e6 100644 --- a/router/tokens/websocket.go +++ b/router/tokens/websocket.go @@ -47,6 +47,36 @@ func DenyForServer(s string, u string) { userDenylist.Store(strings.Join([]string{s, u}, ":"), time.Now()) } +// Checks if a user-bound JWT has been denied because it was issued before Wings +// booted, or before the Panel revoked the user's access to a server. +func isDenylisted(payload *jwt.Payload, serverUUID, userUUID string) bool { + // A token without all of the claims needed to check revocation cannot be + // safely accepted. + if payload.IssuedAt == nil || serverUUID == "" || userUUID == "" { + return true + } + + if payload.IssuedAt.Time.Before(wingsBootTime) { + return true + } + + // This list is deprecated, but we maintain the check here so that custom + // instances are able to continue working. We'll remove it in a future release. + if t, ok := denylist.Load(payload.JWTID); ok { + if !payload.IssuedAt.Time.After(t.(time.Time)) { + return true + } + } + + if t, ok := userDenylist.Load(strings.Join([]string{serverUUID, userUUID}, ":")); ok { + if !payload.IssuedAt.Time.After(t.(time.Time)) { + return true + } + } + + return false +} + // WebsocketPayload defines the JWT payload for a websocket connection. This JWT is passed along to // the websocket after it has been connected to by sending an "auth" event. type WebsocketPayload struct { @@ -79,36 +109,7 @@ func (p *WebsocketPayload) GetServerUuid() string { // before Wings was booted, or because we have denied all tokens with the same JTI // occurring before a set time. func (p *WebsocketPayload) Denylisted() bool { - // If there is no IssuedAt present for the token, we cannot validate the token so - // just immediately mark it as not valid. - if p.IssuedAt == nil { - return true - } - - // If the time that the token was issued is before the time at which Wings was booted - // then the token is invalid for our purposes, even if the token "has permission". - if p.IssuedAt.Time.Before(wingsBootTime) { - return true - } - - // Finally, if the token was issued before a time that is currently denied for this - // token instance, ignore the permissions response. - // - // This list is deprecated, but we maintain the check here so that custom instances - // are able to continue working. We'll remove it in a future release. - if t, ok := denylist.Load(p.JWTID); ok { - if p.IssuedAt.Time.Before(t.(time.Time)) { - return true - } - } - - if t, ok := userDenylist.Load(strings.Join([]string{p.ServerUUID, p.UserUUID}, ":")); ok { - if p.IssuedAt.Time.Before(t.(time.Time)) { - return true - } - } - - return false + return isDenylisted(&p.Payload, p.ServerUUID, p.UserUUID) } // Checks if the given token payload has a permission string.