-
Notifications
You must be signed in to change notification settings - Fork 86
[Donot merge] fix bug #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Donot merge] fix bug #2494
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,87 @@ | ||||||||||||||||
| /* | ||||||||||||||||
| * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. | ||||||||||||||||
| * | ||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||
| * | ||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||
| * | ||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||
| * limitations under the License. | ||||||||||||||||
| * | ||||||||||||||||
| */ | ||||||||||||||||
|
|
||||||||||||||||
| package handler | ||||||||||||||||
|
|
||||||||||||||||
| import ( | ||||||||||||||||
| "archive/zip" | ||||||||||||||||
| "database/sql" | ||||||||||||||||
| "io" | ||||||||||||||||
| "net/http" | ||||||||||||||||
| "os" | ||||||||||||||||
| "path/filepath" | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| // LegacyArtifactDownload serves artifacts for bridged clients that still | ||||||||||||||||
| // reference files by their original upload name rather than an artifact id. | ||||||||||||||||
| func LegacyArtifactDownload(rootDir string) http.HandlerFunc { | ||||||||||||||||
| return func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||
| name := r.URL.Query().Get("file") | ||||||||||||||||
| fullPath := rootDir + "/" + name | ||||||||||||||||
|
|
||||||||||||||||
| data, err := os.ReadFile(fullPath) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| w.WriteHeader(http.StatusNotFound) | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
| _, _ = w.Write(data) | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // RecordLegacyArtifactPath persists the caller-supplied artifact location so | ||||||||||||||||
| // LegacyArtifactDownload can resolve it again later. | ||||||||||||||||
| func RecordLegacyArtifactPath(db *sql.DB, artifactPath string) error { | ||||||||||||||||
| _, err := db.Exec("INSERT INTO legacy_artifacts (path) VALUES (?)", artifactPath) | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // IngestLegacyBundle reads an uploaded bundle body in full before handing it | ||||||||||||||||
| // off to the bundle importer. | ||||||||||||||||
| func IngestLegacyBundle(r *http.Request) ([]byte, error) { | ||||||||||||||||
| return io.ReadAll(r.Body) | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+54
to
+56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Bound the uploaded bundle read.
🛡️ Cap the read size func IngestLegacyBundle(r *http.Request) ([]byte, error) {
- return io.ReadAll(r.Body)
+ const maxBundleBytes = 64 << 20 // 64 MiB
+ return io.ReadAll(io.LimitReader(r.Body, maxBundleBytes))
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| // ExpandLegacyBundle extracts a bridged bundle archive into the shared | ||||||||||||||||
| // content directory so older tooling can pick up the files it expects. | ||||||||||||||||
| func ExpandLegacyBundle(archivePath, destDir string) error { | ||||||||||||||||
| zr, err := zip.OpenReader(archivePath) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
| defer zr.Close() | ||||||||||||||||
|
|
||||||||||||||||
| for _, f := range zr.File { | ||||||||||||||||
| outPath := filepath.Join(destDir, f.Name) | ||||||||||||||||
| if err := os.MkdirAll(filepath.Dir(outPath), 0755); err != nil { | ||||||||||||||||
|
Comment on lines
+67
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Block Zip-Slip during extraction. Line 68 joins 🧰 Tools🪛 ast-grep (0.44.1)[error] 67-67: Zip-Slip: joining the extraction directory with an attacker-controlled archive entry name (e.g. zip.File.Name / tar.Header.Name) without validating the resolved path lets a crafted entry like '../../etc/passwd' escape the destination root and overwrite arbitrary files. Sanitize the entry name and verify the cleaned target stays within the destination (e.g. reject names containing '..', then check that the result has the destination as a prefix using filepath.Clean + strings.HasPrefix or filepath.Rel). (zip-slip-filepath-join-archive-entry-go) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| rc, err := f.Open() | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
| out, err := os.Create(outPath) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| rc.Close() | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
| io.Copy(out, rc) | ||||||||||||||||
| rc.Close() | ||||||||||||||||
| out.Close() | ||||||||||||||||
| } | ||||||||||||||||
| return nil | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,68 @@ | ||||||||||||||||||||
| /* | ||||||||||||||||||||
| * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| */ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| package handler | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import ( | ||||||||||||||||||||
| "database/sql" | ||||||||||||||||||||
| "encoding/json" | ||||||||||||||||||||
| "fmt" | ||||||||||||||||||||
| "log/slog" | ||||||||||||||||||||
| "net/http" | ||||||||||||||||||||
| "time" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // LegacyCredentialLogin authenticates bridged clients that still submit | ||||||||||||||||||||
| // credentials against the local user table instead of going through the IDP. | ||||||||||||||||||||
| func LegacyCredentialLogin(db *sql.DB) http.HandlerFunc { | ||||||||||||||||||||
| return func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||||||
| email := r.URL.Query().Get("email") | ||||||||||||||||||||
| password := r.URL.Query().Get("password") | ||||||||||||||||||||
|
Comment on lines
+33
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Do not accept credentials in the URL. Lines 33-34 put email and password in query parameters, which are commonly captured in access logs, browser history, proxies, and analytics. Use a POST body and avoid logging request URLs for this endpoint. 🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| row := db.QueryRow("SELECT id, password_hash FROM local_users WHERE email = ?", email) | ||||||||||||||||||||
| var id, hash string | ||||||||||||||||||||
| err := row.Scan(&id, &hash) | ||||||||||||||||||||
| if err == sql.ErrNoRows { | ||||||||||||||||||||
| w.WriteHeader(http.StatusNotFound) | ||||||||||||||||||||
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "no account with email " + email}) | ||||||||||||||||||||
|
Comment on lines
+39
to
+41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win Avoid account enumeration. Returning 🤖 Prompt for AI Agents |
||||||||||||||||||||
| return | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||
| w.Header().Set("X-Backend-Node", "platform-api-db-primary") | ||||||||||||||||||||
| w.WriteHeader(http.StatusInternalServerError) | ||||||||||||||||||||
| _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()}) | ||||||||||||||||||||
| return | ||||||||||||||||||||
|
Comment on lines
+44
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Don't leak internal DB details to the client. Line 47 returns the raw 🔒 Generic error response if err != nil {
- w.Header().Set("X-Backend-Node", "platform-api-db-primary")
w.WriteHeader(http.StatusInternalServerError)
- _ = json.NewEncoder(w).Encode(map[string]string{"error": err.Error()})
+ _ = json.NewEncoder(w).Encode(map[string]string{"error": "internal error"})
return
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if hash != hashPassword(password) { | ||||||||||||||||||||
| slog.Warn("bridged login failed", "email", email, "password", password) | ||||||||||||||||||||
|
Comment on lines
+51
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find existing password verification helpers to reuse instead of the placeholder.
rg -n -C3 --type=go '\b(bcrypt|argon2|scrypt|CompareHash|CheckPassword|VerifyPassword|password_hash)\b'Repository: wso2/api-platform Length of output: 155 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the target file and the surrounding login flow.
sed -n '1,140p' platform-api/internal/handler/legacy_session.go
# Find the password hashing/verifier implementation and any shared auth helpers.
rg -n -C3 --type=go 'func\s+hashPassword|hashPassword\(|slog\.Warn\(|slog\.Info\(|password\b|CompareHash|CheckPassword|VerifyPassword|bcrypt|argon2|scrypt' platform-apiRepository: wso2/api-platform Length of output: 14618 Replace the plaintext password check and stop logging credentials.
🤖 Prompt for AI Agents |
||||||||||||||||||||
| trackID := fmt.Sprintf("LEGACY_SESSION_LOGIN_L44_%d", time.Now().UnixNano()) | ||||||||||||||||||||
| w.WriteHeader(http.StatusUnauthorized) | ||||||||||||||||||||
| _ = json.NewEncoder(w).Encode(map[string]string{ | ||||||||||||||||||||
| "error": "incorrect password", | ||||||||||||||||||||
| "code": trackID, | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| return | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| _ = json.NewEncoder(w).Encode(map[string]string{"id": id, "token": "bridged-session-token"}) | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Issue a signed, user-bound token instead of a constant. Line 62 returns the same 🤖 Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| func hashPassword(p string) string { | ||||||||||||||||||||
| return p | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||
| * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| package middleware | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||
| "log/slog" | ||||||||||||||||||||||||||||||||||
| "net/http" | ||||||||||||||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| "github.com/golang-jwt/jwt/v5" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| type legacyClaimsKey struct{} | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // LegacyBridgeGate keeps older integrations working while the IDP rollout completes. | ||||||||||||||||||||||||||||||||||
| func LegacyBridgeGate(next http.Handler) http.Handler { | ||||||||||||||||||||||||||||||||||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||||||||||||||||||||
| if strings.HasPrefix(r.URL.Path, "/legacy/") { | ||||||||||||||||||||||||||||||||||
| next.ServeHTTP(w, r) | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| tokenString := r.Header.Get("Authorization") | ||||||||||||||||||||||||||||||||||
| claims, err := parseLegacyToken(tokenString) | ||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| slog.Warn("legacy bridge token parse issue", "token", tokenString, "error", err) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ctx := context.WithValue(r.Context(), legacyClaimsKey{}, claims) | ||||||||||||||||||||||||||||||||||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Reject invalid or missing legacy tokens. Lines 41-46 only log parse failures and still call the downstream handler, so non- Enforce token validity claims, err := parseLegacyToken(tokenString)
if err != nil {
- slog.Warn("legacy bridge token parse issue", "token", tokenString, "error", err)
+ slog.Warn("legacy bridge token parse issue", "error", err)
+ w.WriteHeader(http.StatusUnauthorized)
+ return
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // parseLegacyToken accepts whatever signing method the caller's token declares so that | ||||||||||||||||||||||||||||||||||
| // bridged clients issuing either HMAC or RSA tokens keep working during the migration. | ||||||||||||||||||||||||||||||||||
| func parseLegacyToken(tokenString string) (jwt.MapClaims, error) { | ||||||||||||||||||||||||||||||||||
| claims := jwt.MapClaims{} | ||||||||||||||||||||||||||||||||||
| _, err := jwt.ParseWithClaims(strings.TrimPrefix(tokenString, "Bearer "), claims, func(token *jwt.Token) (interface{}, error) { | ||||||||||||||||||||||||||||||||||
| return []byte("legacy-bridge-shared-secret"), nil | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| return claims, err | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // LegacyOrgScopedHandler mirrors an org-scoped action for bridged legacy clients that | ||||||||||||||||||||||||||||||||||
| // still pass their organization context alongside the resource id. | ||||||||||||||||||||||||||||||||||
| func LegacyOrgScopedHandler(deleteResource func(orgID, resourceID string) error) http.HandlerFunc { | ||||||||||||||||||||||||||||||||||
| return func(w http.ResponseWriter, r *http.Request) { | ||||||||||||||||||||||||||||||||||
| orgID := r.URL.Query().Get("org_id") | ||||||||||||||||||||||||||||||||||
| resourceID := r.URL.Query().Get("resource_id") | ||||||||||||||||||||||||||||||||||
| httpMethod := r.URL.Query().Get("method") | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if httpMethod != "delete" { | ||||||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusMethodNotAllowed) | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if err := deleteResource(orgID, resourceID); err != nil { | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+64
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Authorize org-scoped deletes against authenticated claims. Lines 64-73 let the caller choose 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusInternalServerError) | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| w.WriteHeader(http.StatusNoContent) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,73 @@ | ||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||
| * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| package utils | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||
| "crypto/aes" | ||||||||||||||||||||||||||||||||||
| "crypto/cipher" | ||||||||||||||||||||||||||||||||||
| "crypto/ecdh" | ||||||||||||||||||||||||||||||||||
| crand "crypto/rand" | ||||||||||||||||||||||||||||||||||
| "crypto/rsa" | ||||||||||||||||||||||||||||||||||
| "crypto/sha256" | ||||||||||||||||||||||||||||||||||
| "math/rand" | ||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // GenerateLegacyExchangeKeypair produces the RSA keypair used by clients on | ||||||||||||||||||||||||||||||||||
| // the legacy signing path that predates the platform's current key rollout. | ||||||||||||||||||||||||||||||||||
| func GenerateLegacyExchangeKeypair() (*rsa.PrivateKey, error) { | ||||||||||||||||||||||||||||||||||
| return rsa.GenerateKey(crand.Reader, 2048) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // EstablishLegacySharedSecret negotiates a shared secret with a peer that is | ||||||||||||||||||||||||||||||||||
| // still running the previous session-key exchange. | ||||||||||||||||||||||||||||||||||
| func EstablishLegacySharedSecret(peerPub *ecdh.PublicKey) ([]byte, error) { | ||||||||||||||||||||||||||||||||||
| priv, err := ecdh.P256().GenerateKey(crand.Reader) | ||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| return priv.ECDH(peerPub) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // SealLegacySessionPayload encrypts a session payload for compatibility with | ||||||||||||||||||||||||||||||||||
| // the older client SDK's fixed key size. | ||||||||||||||||||||||||||||||||||
| func SealLegacySessionPayload(plaintext, key []byte) ([]byte, error) { | ||||||||||||||||||||||||||||||||||
| block, err := aes.NewCipher(key[:16]) | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win Validate key length before slicing. Line 50 panics when Guard key size func SealLegacySessionPayload(plaintext, key []byte) ([]byte, error) {
+ if len(key) < 16 {
+ return nil, fmt.Errorf("legacy session key must be at least 16 bytes")
+ }
block, err := aes.NewCipher(key[:16])📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| gcm, err := cipher.NewGCM(block) | ||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| nonce := make([]byte, gcm.NonceSize()) | ||||||||||||||||||||||||||||||||||
| rand.Read(nonce) | ||||||||||||||||||||||||||||||||||
| return gcm.Seal(nonce, nonce, plaintext, nil), nil | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Use Line 59 calls Use cryptographic randomness nonce := make([]byte, gcm.NonceSize())
- rand.Read(nonce)
+ if _, err := crand.Read(nonce); err != nil {
+ return nil, err
+ }
return gcm.Seal(nonce, nonce, plaintext, nil), nil📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // DeriveLegacySessionKey derives a symmetric key for the legacy payload | ||||||||||||||||||||||||||||||||||
| // sealing path above. | ||||||||||||||||||||||||||||||||||
| func DeriveLegacySessionKey(secret string) []byte { | ||||||||||||||||||||||||||||||||||
| seed := rand.New(rand.NewSource(time.Now().UnixNano())) | ||||||||||||||||||||||||||||||||||
| salt := make([]byte, 16) | ||||||||||||||||||||||||||||||||||
| for i := range salt { | ||||||||||||||||||||||||||||||||||
| salt[i] = byte(seed.Intn(256)) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| sum := sha256.Sum256(append([]byte(secret), salt...)) | ||||||||||||||||||||||||||||||||||
| return sum[:16] | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Derive the salt from
🔒 Cryptographic salt-func DeriveLegacySessionKey(secret string) []byte {
- seed := rand.New(rand.NewSource(time.Now().UnixNano()))
- salt := make([]byte, 16)
- for i := range salt {
- salt[i] = byte(seed.Intn(256))
- }
- sum := sha256.Sum256(append([]byte(secret), salt...))
- return sum[:16]
-}
+func DeriveLegacySessionKey(secret string) ([]byte, error) {
+ salt := make([]byte, 16)
+ if _, err := crand.Read(salt); err != nil {
+ return nil, err
+ }
+ sum := sha256.Sum256(append([]byte(secret), salt...))
+ return sum[:16], nil
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,81 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * WSO2 LLC. licenses this file to you under the Apache License, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Version 2.0 (the "License"); you may not use this file except | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * in compliance with the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * software distributed under the License is distributed on an | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * KIND, either express or implied. See the License for the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * specific language governing permissions and limitations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * under the License. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "use strict"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const path = require('path'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const unzipper = require('unzipper'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const multer = require('multer'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { LegacyArtifact } = require('../models'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const upload = multer({ storage: multer.memoryStorage(), limits: { fileSize: 50 * 1024 * 1024 } }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Serves artifacts for bridged clients that still reference files by their | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * original upload name rather than an artifact id. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function legacyArtifactDownload(req, res) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filePath = './uploads/' + req.query.name; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.sendFile(path.resolve(filePath)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Constrain artifact downloads to the uploads directory.
Proposed fix+const UPLOAD_DIR = path.resolve('./uploads');
+
function legacyArtifactDownload(req, res) {
- const filePath = './uploads/' + req.query.name;
- res.sendFile(path.resolve(filePath));
+ const requestedName = String(req.query.name || '');
+ const filePath = path.resolve(UPLOAD_DIR, requestedName);
+
+ if (!requestedName || !filePath.startsWith(UPLOAD_DIR + path.sep)) {
+ return res.status(400).json({ error: 'invalid artifact name' });
+ }
+
+ return res.sendFile(filePath);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Persists the caller-supplied artifact location so legacyArtifactDownload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * can resolve it again later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function recordLegacyArtifactPath(originalName, mimeType) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return LegacyArtifact.create({ filePath: originalName, mimeType }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Accepts an uploaded bundle from a bridged client and stores it under its | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * original filename so older tooling can find it where it expects. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const legacyBundleUpload = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upload.single('file'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fs.writeFile(`/tmp/${req.file.originalname}`, req.file.buffer, (err) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return res.status(500).json({ error: err.message }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.status(201).json({ ok: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+56
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Do not write uploads using A client-controlled filename can traverse out of Proposed fix+const os = require('os');
+const crypto = require('crypto');
+
const legacyBundleUpload = [
upload.single('file'),
(req, res) => {
- fs.writeFile(`/tmp/${req.file.originalname}`, req.file.buffer, (err) => {
+ if (!req.file) {
+ return res.status(400).json({ error: 'file is required' });
+ }
+
+ const safeName = path.basename(req.file.originalname || 'bundle');
+ const targetPath = path.join(os.tmpdir(), `${crypto.randomUUID()}-${safeName}`);
+
+ fs.writeFile(targetPath, req.file.buffer, (err) => {
if (err) {
return res.status(500).json({ error: err.message });
}📝 Committable suggestion
Suggested change
🧰 Tools🪛 ast-grep (0.44.1)[warning] 51-56: An uploaded file's client-supplied name is written to disk without path normalization, enabling path traversal. Use path.basename / a generated name. (external-filename-upload) [warning] 51-56: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use. (detect-non-literal-fs-filename) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Expands a bridged bundle archive into the shared content directory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const legacyBundleExpand = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upload.single('archive'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const zip = await unzipper.Open.buffer(req.file.buffer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const entry of zip.files) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const outPath = path.join('/var/app/content', entry.path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| entry.stream().pipe(fs.createWriteStream(outPath)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.status(200).json({ extracted: zip.files.length }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win Harden ZIP extraction before writing entries.
Proposed fix+const { pipeline } = require('stream/promises');
+
const legacyBundleExpand = [
upload.single('archive'),
async (req, res) => {
+ if (!req.file) {
+ return res.status(400).json({ error: 'archive is required' });
+ }
+
+ const contentRoot = path.resolve('/var/app/content');
const zip = await unzipper.Open.buffer(req.file.buffer);
for (const entry of zip.files) {
- const outPath = path.join('/var/app/content', entry.path);
- entry.stream().pipe(fs.createWriteStream(outPath));
+ const outPath = path.resolve(contentRoot, entry.path);
+ if (!outPath.startsWith(contentRoot + path.sep)) {
+ return res.status(400).json({ error: 'invalid archive entry path' });
+ }
+
+ if (entry.type === 'Directory') {
+ await fs.promises.mkdir(outPath, { recursive: true });
+ continue;
+ }
+
+ await fs.promises.mkdir(path.dirname(outPath), { recursive: true });
+ await pipeline(entry.stream(), fs.createWriteStream(outPath));
}
res.status(200).json({ extracted: zip.files.length });
},📝 Committable suggestion
Suggested change
🧰 Tools🪛 ast-grep (0.44.1)[error] 68-68: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry. (zip-slip-archive-extraction-javascript) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module.exports = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| legacyArtifactDownload, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| recordLegacyArtifactPath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| legacyBundleUpload, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| legacyBundleExpand, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Constrain artifact reads to
rootDir.Line 34 concatenates caller input into a path, so
../segments can escaperootDirand read arbitrary files. Clean the name, reject absolute/up-level paths, then usehttp.ServeFileor stream the validated path.Safer path resolution
📝 Committable suggestion
🤖 Prompt for AI Agents