Skip to content

Commit 979624e

Browse files
committed
Fixed weird edgecases with sanitisation and usage of protected keys
1 parent 519e272 commit 979624e

File tree

2 files changed

+24
-15
lines changed

2 files changed

+24
-15
lines changed

db-connector.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13345,7 +13345,7 @@ func SetDatastoreKeyBulk(ctx context.Context, allKeys []CacheKeyData) ([]Datasto
1334513345

1334613346
// URL encode
1334713347
datastoreId = url.QueryEscape(datastoreId)
13348-
if len(cacheData.PublicAuthorization) == 0 {
13348+
if len(cacheData.PublicAuthorization) == 0 && cacheData.Category != "protected" {
1334913349
cacheData.PublicAuthorization = uuid.NewV4().String()
1335013350
}
1335113351

@@ -13763,7 +13763,7 @@ func SetDatastoreKeyRevision(ctx context.Context, cacheData CacheKeyData) error
1376313763
}
1376413764

1376513765
cacheData.Authorization = ""
13766-
if len(cacheData.PublicAuthorization) == 0 {
13766+
if len(cacheData.PublicAuthorization) == 0 && cacheData.Category != "protected" {
1376713767
cacheData.PublicAuthorization = uuid.NewV4().String()
1376813768
}
1376913769

@@ -13829,7 +13829,7 @@ func SetDatastoreKey(ctx context.Context, cacheData CacheKeyData) error {
1382913829
}
1383013830

1383113831
cacheData.Authorization = ""
13832-
if len(cacheData.PublicAuthorization) == 0 {
13832+
if len(cacheData.PublicAuthorization) == 0 && cacheData.Category != "protected" {
1383313833
cacheData.PublicAuthorization = uuid.NewV4().String()
1383413834
}
1383513835

@@ -13956,7 +13956,9 @@ func GetDatastoreKey(ctx context.Context, id string, category string) (*CacheKey
1395613956

1395713957
category = strings.ReplaceAll(strings.ToLower(category), " ", "_")
1395813958
if len(category) > 0 && category != "default" {
13959-
if !strings.HasSuffix(id, category) {
13959+
// FIXME: If they key itself is 'test_protected' and category
13960+
// is 'protected' this breaks... Keeping it for now.
13961+
if !strings.HasSuffix(id, fmt.Sprintf("_%s", category)) {
1396013962
id = fmt.Sprintf("%s_%s", id, category)
1396113963
}
1396213964
}
@@ -14016,9 +14018,8 @@ func GetDatastoreKey(ctx context.Context, id string, category string) (*CacheKey
1401614018
cacheData = &wrapped.Source
1401714019
} else {
1401814020
key := datastore.NameKey(nameKey, id, nil)
14019-
1402014021
if err := project.Dbclient.Get(ctx, key, cacheData); err != nil {
14021-
//log.Printf("ERROR: Failed getting cache key %s: %s", id, err)
14022+
//log.Printf("[WARNING]: Failed getting cache key %s: %s", id, err)
1402214023

1402314024
if strings.Contains(err.Error(), `cannot load field`) {
1402414025
log.Printf("[ERROR] Error in cache key loading. Migrating org cache to new handler (3): %s", err)
@@ -14089,7 +14090,9 @@ func GetDatastoreKey(ctx context.Context, id string, category string) (*CacheKey
1408914090
newValue, err := HandleKeyDecryption([]byte(cacheData.Value), encryptionKey)
1409014091
if err == nil {
1409114092
cacheData.Value = string(newValue)
14092-
cacheData.Encrypted = false
14093+
14094+
// Not removing this as it just causes confusion
14095+
//cacheData.Encrypted = false
1409314096
}
1409414097
}
1409514098

shared.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20028,7 +20028,7 @@ func HandleGetCacheKey(resp http.ResponseWriter, request *http.Request) {
2002820028
cacheId := fmt.Sprintf("%s_%s", tmpData.OrgId, tmpData.Key)
2002920029
cacheData, err := GetDatastoreKey(ctx, cacheId, tmpData.Category)
2003020030
if err != nil {
20031-
20031+
log.Printf("[WARNING] Failed to GET cache key '%s' for org %s (get)", tmpData.Key, tmpData.OrgId)
2003220032
// Doing a last resort search, e.g. to handle spaces and the like
2003320033
allkeys, _, err := GetAllCacheKeys(ctx, org.Id, "", 150, "")
2003420034
if err == nil {
@@ -20063,7 +20063,7 @@ func HandleGetCacheKey(resp http.ResponseWriter, request *http.Request) {
2006320063
}
2006420064
}
2006520065

20066-
if len(cacheData.PublicAuthorization) == 0 {
20066+
if len(cacheData.PublicAuthorization) == 0 && cacheData.Category != "protected" {
2006720067
cacheId := fmt.Sprintf("%s_%s", tmpData.OrgId, tmpData.Key)
2006820068
if len(tmpData.Category) > 0 && tmpData.Category != "default" {
2006920069
cacheId = fmt.Sprintf("%s_%s", cacheId, tmpData.Category)
@@ -33065,9 +33065,11 @@ func cleanupProtectedKeys(exec WorkflowExecution) WorkflowExecution {
3306533065
return exec
3306633066
}
3306733067

33068-
if exec.Status == "FINISHED" || exec.Status == "ABORTED" {
33069-
return exec
33070-
}
33068+
// This doesn't matter as we are checking for 'sanitized' anyway
33069+
// This also makes it stop too early
33070+
//if exec.Status == "FINISHED" || exec.Status == "ABORTED" {
33071+
// return exec
33072+
//}
3307133073

3307233074
protectedKeys, _, err := GetAllCacheKeys(context.Background(), exec.ExecutionOrg, "protected", 100, "")
3307333075
if err != nil {
@@ -33076,11 +33078,11 @@ func cleanupProtectedKeys(exec WorkflowExecution) WorkflowExecution {
3307633078
}
3307733079

3307833080
for resultKey, _ := range exec.Results {
33079-
if exec.Results[resultKey].Status != "FINISHED" && exec.Results[resultKey].Status != "SUCCESS" {
33081+
if exec.Results[resultKey].Sanitized {
3308033082
continue
3308133083
}
3308233084

33083-
if exec.Results[resultKey].Sanitized {
33085+
if exec.Results[resultKey].Status != "FINISHED" && exec.Results[resultKey].Status != "SUCCESS" {
3308433086
continue
3308533087
}
3308633088

@@ -33091,7 +33093,11 @@ func cleanupProtectedKeys(exec WorkflowExecution) WorkflowExecution {
3309133093
} else if len(protectedKey.Value) > 2000000 {
3309233094
exec.Results[resultKey].Result = strings.ReplaceAll(exec.Results[resultKey].Result, protectedKey.Value, "***")
3309333095
} else {
33094-
exec.Results[resultKey].Result = SanitizeFuzzySubstring(exec.Results[resultKey].Result, protectedKey.Value, 2)
33096+
exec.Results[resultKey].Result = strings.ReplaceAll(exec.Results[resultKey].Result, protectedKey.Value, "***")
33097+
33098+
// FIXME: Should do more fuzzy sanitizing, but there are too
33099+
// many edgecases for it
33100+
//exec.Results[resultKey].Result = SanitizeFuzzySubstring(exec.Results[resultKey].Result, protectedKey.Value, 2)
3309533101
}
3309633102
}
3309733103

0 commit comments

Comments
 (0)