Skip to content

Commit 468676a

Browse files
MartKazakCopilot
MartKazak
and
Copilot
authored
FUND-2081 - Improvements regarding Polaris scan (#95)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 1a0f83a commit 468676a

File tree

5 files changed

+135
-144
lines changed

5 files changed

+135
-144
lines changed

‎src/OneGround.ZGW.Common/Helpers/TempFileHelper.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,22 @@ namespace OneGround.ZGW.Common.Helpers;
55

66
public static class TempFileHelper
77
{
8-
// Note: Attempt to fix issue 'Filesystem path, filename, or URI manipulation'
9-
public static void AssureNotTampered(string fullFileName)
8+
public static string GetValidatedPath(string fullFileName)
109
{
1110
if (fullFileName == null)
12-
return;
11+
return null;
1312

1413
if (fullFileName.Contains(".."))
1514
throw new SecurityException("Full file name contains not allowed characters.");
1615

1716
var directory = Path.GetDirectoryName(fullFileName);
1817

1918
if (directory == null)
20-
return;
19+
return null;
2120

2221
if (!directory.StartsWith(Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar)))
23-
throw new SecurityException("Temporary file does not contains the temporary folder which should be.");
22+
throw new SecurityException("Temporary file does not contain the temporary folder which should be.");
23+
24+
return fullFileName;
2425
}
2526
}

‎src/OneGround.ZGW.Documenten.Services.Ceph/CephDocumentServices.cs‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ public class CephDocumentServices : IDocumentService
2828
public CephDocumentServices(ILogger<CephDocumentServices> logger, IConfiguration configuration)
2929
{
3030
_logger = logger;
31-
3231
_lazySettings = new Lazy<CephDocumentServicesSettings>(() => GetSettings(configuration));
33-
3432
_lazyClient = new Lazy<AmazonS3Client>(GetClient);
3533
}
3634

@@ -58,23 +56,30 @@ public async Task<Document> AddDocumentAsync(
5856
if (metadata == null || string.IsNullOrEmpty(metadata.Rsin))
5957
throw new InvalidOperationException("Rsin is required to be filled in metadata");
6058

61-
TempFileHelper.AssureNotTampered(contentFile);
59+
var validatedContentFile = TempFileHelper.GetValidatedPath(contentFile);
6260

63-
if (!File.Exists(contentFile))
64-
throw new InvalidOperationException($"Content file '{contentFile}' does not exist.");
61+
if (!File.Exists(validatedContentFile))
62+
throw new InvalidOperationException($"Content file '{validatedContentFile}' does not exist.");
6563

66-
string bucket = GetOrGenerateBucketName(metadata.Rsin);
64+
var bucket = GetOrGenerateBucketName(metadata.Rsin);
6765

6866
await EnsureBucketExistsAsync(bucket, cancellationToken);
6967

7068
var key = $"{Guid.NewGuid()}";
7169

7270
try
7371
{
74-
using FileStream content = new FileStream(contentFile, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize, useAsync: true); // When using 'useAsync: true' you get better performance with buffers much larger than the default 4096 bytes.
75-
long documentSize = content.Length;
76-
77-
_logger.LogDebug("Streaming data from {contentFile} to AmazonS3 object using key {key}...", contentFile, key);
72+
await using var content = new FileStream(
73+
validatedContentFile,
74+
FileMode.Open,
75+
FileAccess.Read,
76+
FileShare.Read,
77+
bufferSize,
78+
useAsync: true
79+
); // When using 'useAsync: true' you get better performance with buffers much larger than the default 4096 bytes.
80+
var documentSize = content.Length;
81+
82+
_logger.LogDebug("Streaming data from {validatedContentFile} to AmazonS3 object using key {key}...", validatedContentFile, key);
7883

7984
var request = new PutObjectRequest
8085
{
@@ -85,7 +90,7 @@ public async Task<Document> AddDocumentAsync(
8590

8691
request.Metadata.Add(metadata, documentName);
8792

88-
using (request.InputStream = content)
93+
await using (request.InputStream = content)
8994
{
9095
_logger.LogDebug("Writing document '{documentName}' to bucket '{bucket}'...", documentName, bucket);
9196

@@ -104,7 +109,7 @@ public async Task<Document> AddDocumentAsync(
104109
{
105110
if (deleteContentFile)
106111
{
107-
File.Delete(contentFile);
112+
File.Delete(validatedContentFile);
108113
}
109114
}
110115
}
@@ -125,13 +130,13 @@ public async Task<Document> AddDocumentAsync(
125130
if (metadata == null || string.IsNullOrEmpty(metadata.Rsin))
126131
throw new InvalidOperationException("Rsin is required to be filled in metadata");
127132

128-
string bucket = GetOrGenerateBucketName(metadata.Rsin);
133+
var bucket = GetOrGenerateBucketName(metadata.Rsin);
129134

130135
await EnsureBucketExistsAsync(bucket, cancellationToken);
131136

132137
var key = $"{Guid.NewGuid()}";
133138

134-
long length = content.Length;
139+
var length = content.Length;
135140

136141
var request = new PutObjectRequest
137142
{
@@ -160,7 +165,7 @@ public async Task DeleteDocumentAsync(DocumentUrn urn, CancellationToken cancell
160165

161166
if (await DocumentExistsAsync(urn, cancellationToken))
162167
{
163-
DeleteObjectRequest request = new DeleteObjectRequest { BucketName = urn.Name, Key = urn.ObjectId };
168+
var request = new DeleteObjectRequest { BucketName = urn.Name, Key = urn.ObjectId };
164169

165170
await Client.DeleteObjectAsync(request, cancellationToken);
166171
}
@@ -213,7 +218,7 @@ public async Task<MultiPartDocument> InitiateMultipartUploadAsync(
213218
if (metadata == null || string.IsNullOrEmpty(metadata.Rsin))
214219
throw new InvalidOperationException("Rsin is required to be filled in metadata");
215220

216-
string bucket = GetOrGenerateBucketName(metadata.Rsin);
221+
var bucket = GetOrGenerateBucketName(metadata.Rsin);
217222

218223
await EnsureBucketExistsAsync(bucket, cancellationToken);
219224

@@ -230,7 +235,7 @@ public async Task<MultiPartDocument> InitiateMultipartUploadAsync(
230235

231236
var response = await Client.InitiateMultipartUploadAsync(request, cancellationToken);
232237

233-
InternalMultiPartDocument internalMultiPartDocument = new InternalMultiPartDocument(response.UploadId, bucket, key);
238+
var internalMultiPartDocument = new InternalMultiPartDocument(response.UploadId, bucket, key);
234239

235240
return ToContext(internalMultiPartDocument);
236241
}
@@ -245,7 +250,7 @@ public async Task<IUploadPart> TryUploadPartAsync(
245250
{
246251
LastError = ("", DocumentError.None);
247252

248-
InternalMultiPartDocument internalMultiPartDocument = FromContext(multipPartDocument);
253+
var internalMultiPartDocument = FromContext(multipPartDocument);
249254

250255
if (content.Length != size)
251256
{
@@ -282,7 +287,7 @@ public async Task<Document> CompleteMultipartUploadAsync(
282287
{
283288
LastError = ("", DocumentError.None);
284289

285-
InternalMultiPartDocument internalMultiPartDocument = FromContext(multipPartDocument);
290+
var internalMultiPartDocument = FromContext(multipPartDocument);
286291

287292
var parts = uploadparts.Select(MapUploadPart).ToList();
288293

@@ -300,7 +305,7 @@ public async Task<Document> CompleteMultipartUploadAsync(
300305

301306
var documentUrn = new DocumentUrn(ProviderPrefix, internalMultiPartDocument.Name, internalMultiPartDocument.Key);
302307

303-
long length = parts.Sum(p => p.Size);
308+
var length = parts.Sum(p => p.Size);
304309

305310
return new Document(documentUrn, length);
306311
}
@@ -311,7 +316,7 @@ public async Task<bool> AbortMultipartUploadAsync(IMultiPartDocument multipPartD
311316

312317
try
313318
{
314-
InternalMultiPartDocument internalMultiPartDocument = FromContext(multipPartDocument);
319+
var internalMultiPartDocument = FromContext(multipPartDocument);
315320

316321
var request = new AbortMultipartUploadRequest
317322
{
@@ -429,20 +434,20 @@ private AmazonS3Client GetClient()
429434

430435
private string GetOrGenerateBucketName(string rsin)
431436
{
432-
string buckettemplate = Settings.Bucket ?? "{yyyyMM}"; // Note: When not specified default is: store buckets per year+month
437+
var buckettemplate = Settings.Bucket ?? "{yyyyMM}"; // Note: When not specified default is: store buckets per year+month
433438

434439
buckettemplate = buckettemplate.Replace("{rsin}", rsin);
435440

436-
int posTmplL = buckettemplate.IndexOf('{');
437-
int posTmplR = buckettemplate.IndexOf('}');
441+
var posTmplL = buckettemplate.IndexOf('{');
442+
var posTmplR = buckettemplate.IndexOf('}');
438443

439444
string bucket;
440445
if (posTmplL > -1 && posTmplR > -1 && posTmplL < posTmplR)
441446
{
442447
var template = buckettemplate.Substring(posTmplL + 1, posTmplR - posTmplL - 1);
443448

444-
string leftPart = buckettemplate.Substring(0, posTmplL);
445-
string rightPart = buckettemplate.Substring(posTmplR + 1);
449+
var leftPart = buckettemplate.Substring(0, posTmplL);
450+
var rightPart = buckettemplate.Substring(posTmplR + 1);
446451

447452
bucket = leftPart + DateTime.UtcNow.ToString(template) + rightPart;
448453
}
@@ -462,7 +467,7 @@ private string GetOrGenerateBucketName(string rsin)
462467

463468
private static CephDocumentServicesSettings GetSettings(IConfiguration configuration)
464469
{
465-
string key = "CephDocumentServicesSettings";
470+
var key = "CephDocumentServicesSettings";
466471

467472
var settings =
468473
configuration.GetSection(key).Get<CephDocumentServicesSettings>()

‎src/Tests/OneGround.ZGW.Catalogi.WebApi.UnitTests/BusinessRulesTests/ConceptBusinessRulesTest.cs‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using Moq;
77
using OneGround.ZGW.Catalogi.DataModel;
88
using OneGround.ZGW.Catalogi.Web.BusinessRules;
9-
using OneGround.ZGW.Common.Contracts.v1;
109
using OneGround.ZGW.Common.Web.Services.UriServices;
1110
using Xunit;
1211

@@ -96,11 +95,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Missing_Original
9695
{
9796
// Setup
9897

99-
var existingZaakTypeGuids = new List<Guid>
100-
{
101-
new Guid("16ee54a6-5578-16e0-014d-2ee79069bb55"),
102-
new Guid("835827db-e46d-4d84-a41c-ccd80c3f9e4a"),
103-
};
98+
var existingZaakTypeGuids = new List<Guid> { new("16ee54a6-5578-16e0-014d-2ee79069bb55"), new("835827db-e46d-4d84-a41c-ccd80c3f9e4a") };
10499

105100
var patchZaakTypeUrls = new List<string>
106101
{
@@ -110,7 +105,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Missing_Original
110105
};
111106

112107
// Act
113-
bool validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
108+
var validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
114109

115110
// Assert
116111
Assert.False(validated);
@@ -121,11 +116,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Original_Relatio
121116
{
122117
// Setup
123118

124-
var existingZaakTypeGuids = new List<Guid>
125-
{
126-
new Guid("f6d279e0-3bcb-4263-9585-6448a3690a39"),
127-
new Guid("835827db-e46d-4d84-a41c-ccd80c3f9e4a"),
128-
};
119+
var existingZaakTypeGuids = new List<Guid> { new("f6d279e0-3bcb-4263-9585-6448a3690a39"), new("835827db-e46d-4d84-a41c-ccd80c3f9e4a") };
129120

130121
var patchZaakTypeUrls = new List<string>
131122
{
@@ -139,7 +130,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Original_Relatio
139130
_mockedUriService.Setup(m => m.GetId(patchZaakTypeUrls[2])).Returns(Guid.Parse(patchZaakTypeUrls[2].Split('/').Last()));
140131

141132
// Act
142-
bool validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
133+
var validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
143134

144135
// Assert
145136
Assert.True(validated);
@@ -150,11 +141,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Original_Relatio
150141
{
151142
// Setup
152143

153-
var existingZaakTypeGuids = new List<Guid>
154-
{
155-
new Guid("f6d279e0-3bcb-4263-9585-6448a3690a39"),
156-
new Guid("835827db-e46d-4d84-a41c-ccd80c3f9e4a"),
157-
};
144+
var existingZaakTypeGuids = new List<Guid> { new("f6d279e0-3bcb-4263-9585-6448a3690a39"), new("835827db-e46d-4d84-a41c-ccd80c3f9e4a") };
158145

159146
var patchZaakTypeUrls = new List<string>
160147
{
@@ -168,7 +155,7 @@ public void BesluitTypeRelationsValidator_Add_New_Relation_With_Original_Relatio
168155
_mockedUriService.Setup(m => m.GetId(patchZaakTypeUrls[2])).Returns(Guid.Parse(patchZaakTypeUrls[2].Split('/').Last()));
169156

170157
// Act
171-
bool validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
158+
var validated = _besluitTypeRelationsValidator.Validate(existingZaakTypeGuids, patchZaakTypeUrls);
172159

173160
// Assert
174161
Assert.True(validated);

‎src/Tests/OneGround.ZGW.Common.UnitTests/TempFileHelperTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void AssureNotTampered_With_An_Valid_Temp_File_Should_Return()
1818
// Act
1919
var ex = Record.Exception(() =>
2020
{
21-
TempFileHelper.AssureNotTampered(fullFileName: fullFileName);
21+
TempFileHelper.GetValidatedPath(fullFileName: fullFileName);
2222
});
2323

2424
// Assert
@@ -34,7 +34,7 @@ public void AssureNotTampered_With_An_Tampered_File_Should_Throw_An_Exception()
3434
// Act
3535
var ex = Record.Exception(() =>
3636
{
37-
TempFileHelper.AssureNotTampered(fullFileName: @"c:\windows\system32\kernel32.dll");
37+
TempFileHelper.GetValidatedPath(fullFileName: @"c:\windows\system32\kernel32.dll");
3838
});
3939

4040
// Assert
@@ -54,7 +54,7 @@ public void AssureNotTampered_With_An_Tampered_Path_Should_Throw_An_Exception()
5454
// Act
5555
var ex = Record.Exception(() =>
5656
{
57-
TempFileHelper.AssureNotTampered(fullFileName: fullFileName);
57+
TempFileHelper.GetValidatedPath(fullFileName: fullFileName);
5858
});
5959

6060
// Assert

0 commit comments

Comments
 (0)