[backend/pycti] Prevent file upload looping (#14585) by richard-julien · Pull Request #14749 · OpenCTI-Platform/opencti

richard-julien · 2026-03-03T22:43:32Z

codecov · 2026-03-03T23:43:20Z

Codecov Report

❌ Patch coverage is 21.73913% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.42%. Comparing base (e22f632) to head (186f19f).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
...playbook/components/container-wrapper-component.ts	19.04%	17 Missing ⚠️
...tform/opencti-graphql/src/database/draft-engine.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #14749      +/-   ##
==========================================
+ Coverage   32.35%   32.42%   +0.06%     
==========================================
  Files        3107     3108       +1     
  Lines      211587   211869     +282     
  Branches    38361    38470     +109     
==========================================
+ Hits        68469    68694     +225     
- Misses     143118   143175      +57

Flag	Coverage Δ
opencti-client-python	`45.53% <ø> (+0.04%)`	⬆️
opencti-front	`2.82% <ø> (-0.01%)`	⬇️
opencti-graphql	`67.74% <21.73%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR addresses issue #14585 where playbook-driven “knowledge manipulation” updates on containers inadvertently delete/re-add existing attached files, causing noisy history entries and unnecessary file operations.

Changes:

Backend: when the Container Wrapper component is configured to copy files, it now attempts to embed file content (data) into the STIX file extensions instead of copying only uri references.
PyCTI: removes the “fetch file by uri” fallback during STIX import, so files are only uploaded when data is explicitly embedded.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts	Updates container wrapping to resolve and embed file contents when `copyFiles` is enabled.
client-python/pycti/utils/opencti_stix2.py	Prevents implicit file re-download/re-upload by requiring embedded `data` for file import.

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

client-python/pycti/utils/opencti_stix2.py

aHenryJard · 2026-03-04T10:56:00Z

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

+            const currentFileUri = currentFile.uri;
+            const fileId = currentFileUri.replace('/storage/get/', '');
+            const currentFileContent = toBase64(await getFileContent(fileId));
+            copiedFiles.push({ ...currentFile, data: currentFileContent });


Don't we need the file markings in addition of the file content ?

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Copilot · 2026-03-04T12:40:03Z

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

+            const currentFileUri = currentFile.uri;
+            const fileId = currentFileUri.replace('/storage/get/', '');
+            const currentFileContent = await getFileContent(fileId, 'base64');
+            if (currentFileContent) {
+              copiedFiles.push({ ...currentFile, data: currentFileContent });
+            } else {
+              logApp.error('Cant copy file from main element to the container: empty content', { name: currentFile.name });
+            }
+          } catch (e) {
+            logApp.error('Cant copy file from main element to the container', { cause: e, name: currentFile.name });
+          }
+        }
+        container.extensions[STIX_EXT_OCTI].files = copiedFiles;


When copying STIX file extensions and embedding data, we should also propagate/set no_trigger_import: true (same as convertStoreToStixWithResolvedFiles does) to avoid triggering automatic import/enrichment jobs for these re-uploaded attachments and potentially reintroducing loops.

Copilot · 2026-03-04T12:40:03Z

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

+          try {
+            const currentFileUri = currentFile.uri;


This assumes currentFile.uri is always a /storage/get/<id> path. In STIX 2.1 conversion, files that already include data often have uri: 'unknown'; in that case this code will try to fetch getFileContent('unknown', ...) and log errors. Consider short-circuiting when currentFile.data is already present (or when uri is missing/unknown) and just reuse the existing data.

Suggested change

try {

const currentFileUri = currentFile.uri;

try {

// If the file already contains data (common in STIX 2.1 bundles), reuse it directly.

if (currentFile.data) {

copiedFiles.push({ ...currentFile });

// eslint-disable-next-line no-continue

continue;

}

const currentFileUri = currentFile.uri;

// In some STIX 2.1 conversions, uri can be 'unknown' or missing; avoid bogus storage lookups.

if (!currentFileUri || currentFileUri === 'unknown') {

logApp.warn('Cant copy file from main element to the container: invalid or missing uri', {

name: currentFile.name,

uri: currentFileUri,

});

// eslint-disable-next-line no-continue

continue;

}

if (!currentFileUri.startsWith('/storage/get/')) {

logApp.warn('Cant copy file from main element to the container: unexpected uri format', {

name: currentFile.name,

uri: currentFileUri,

});

// eslint-disable-next-line no-continue

continue;

}

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

Copilot · 2026-03-04T12:40:04Z

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

+      const stixFileExtensions = baseData.extensions[STIX_EXT_OCTI].files;
+      if (copyFiles && stixFileExtensions && stixFileExtensions.length > 0) {
+        // We need to get the files and add the data inside
+        const copiedFiles = [];
+        for (let index = 0; index < stixFileExtensions.length; index += 1) {
+          const currentFile = stixFileExtensions[index];
+          try {
+            const currentFileUri = currentFile.uri;
+            const fileId = currentFileUri.replace('/storage/get/', '');
+            const currentFileContent = await getFileContent(fileId, 'base64');
+            if (currentFileContent) {
+              copiedFiles.push({ ...currentFile, data: currentFileContent });
+            } else {
+              logApp.error('Cant copy file from main element to the container: empty content', { name: currentFile.name });
+            }
+          } catch (e) {
+            logApp.error('Cant copy file from main element to the container', { cause: e, name: currentFile.name });
+          }
+        }
+        container.extensions[STIX_EXT_OCTI].files = copiedFiles;


The new copyFiles path now resolves each file's content from storage and injects it into the STIX file extension. There are existing integration tests for PLAYBOOK_CONTAINER_WRAPPER_COMPONENT, but none covering copyFiles: true; please add a test asserting that when copyFiles is enabled, container.extensions[STIX_EXT_OCTI].files includes data (and any required flags like no_trigger_import).

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

xfournet · 2026-03-04T14:57:34Z

opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts

+            const currentFileUri = currentFile.uri;
+            const fileId = currentFileUri.replace('/storage/get/', '');
+            const currentFileContent = await getFileContent(fileId, 'base64');
+            if (currentFileContent) {


empty file (probably a usage error) should not trigger an error from copy logic perspective. we should log error only if file is not found ?

aHenryJard

Tested locally on report update with file attach and one that has a dedicated marking
=> use a playbook to copy report content into incident and it has worked as expected.

Tested also on report without files attached

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-04T19:21:59Z

opencti-platform/opencti-graphql/src/modules/playbook/components/container-wrapper-component.ts

+            if (currentFile.data) {
+              copiedFiles.push({ ...currentFile, no_trigger_import: true });
+            } else {
+              // If data not in the element, fetch it in base64
+              const currentFileUri = currentFile.uri;
+              const fileId = currentFileUri.replace('/storage/get/', '');
+              const currentFileContent = await getFileContent(fileId, 'base64');
+              if (currentFileContent) { // File does not exist anymore


The checks if (currentFile.data) and if (currentFileContent) rely on truthiness, so an empty (but valid) base64 string (e.g., a 0‑byte file) will be treated as “missing” and either re-fetched or skipped, which can lead to attachments being dropped from the container. Use explicit null/undefined checks instead (e.g., test for currentFile.data !== undefined and currentFileContent !== undefined).

Suggested change

if (currentFile.data) {

copiedFiles.push({ ...currentFile, no_trigger_import: true });

} else {

// If data not in the element, fetch it in base64

const currentFileUri = currentFile.uri;

const fileId = currentFileUri.replace('/storage/get/', '');

const currentFileContent = await getFileContent(fileId, 'base64');

if (currentFileContent) { // File does not exist anymore

if (currentFile.data !== undefined && currentFile.data !== null) {

copiedFiles.push({ ...currentFile, no_trigger_import: true });

} else {

// If data not in the element, fetch it in base64

const currentFileUri = currentFile.uri;

const fileId = currentFileUri.replace('/storage/get/', '');

const currentFileContent = await getFileContent(fileId, 'base64');

if (currentFileContent !== undefined && currentFileContent !== null) { // File does not exist anymore

@richard-julien same remark for me. The copilot suggestion is good

opencti-platform/opencti-graphql/src/modules/playbook/components/container-wrapper-component.ts

Copilot · 2026-03-04T19:22:00Z

opencti-platform/opencti-graphql/src/modules/playbook/components/container-wrapper-component.ts

+              if (currentFileContent) { // File does not exist anymore
+                copiedFiles.push({ ...currentFile, data: currentFileContent, no_trigger_import: true });
+              }
+            }
+          } catch (e) {
+            logApp.error("Can't copy file from main element to the container", { cause: e, name: currentFile.name });
+          }


Inline comment // File does not exist anymore is misleading here: getFileContent() can throw on missing S3 keys, and the if (currentFileContent) condition is about truthiness (also false for empty content), not existence. Please update the comment and/or handle the missing-file case explicitly (e.g., catching NoSuchKey and treating it as a non-error skip).

Suggested change

if (currentFileContent) { // File does not exist anymore

copiedFiles.push({ ...currentFile, data: currentFileContent, no_trigger_import: true });

}

}

} catch (e) {

logApp.error("Can't copy file from main element to the container", { cause: e, name: currentFile.name });

}

// Only copy the file if some (truthy) content was retrieved; missing-file errors are handled by the try/catch.

if (currentFileContent) {

copiedFiles.push({ ...currentFile, data: currentFileContent, no_trigger_import: true });

}

}

} catch (e) {

logApp.error("Can't copy file from main element to the container", { cause: e, name: currentFile.name });

richard-julien linked an issue Mar 3, 2026 that may be closed by this pull request

Playbook: manipulating container files even if not needed #14585

Closed

richard-julien added the filigran team use to identify PR from the Filigran team label Mar 4, 2026

richard-julien requested a review from Copilot March 4, 2026 08:28

Copilot started reviewing on behalf of richard-julien March 4, 2026 08:29 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

aHenryJard reviewed Mar 4, 2026

View reviewed changes

richard-julien requested a review from Copilot March 4, 2026 12:35

Copilot started reviewing on behalf of richard-julien March 4, 2026 12:35 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

xfournet reviewed Mar 4, 2026

View reviewed changes

aHenryJard approved these changes Mar 4, 2026

View reviewed changes

richard-julien force-pushed the issue/14585_loop branch from 1c346dd to 845696c Compare March 4, 2026 17:39

richard-julien added 2 commits March 4, 2026 18:39

[backend/pycti] Prevent file upload looping (#14585)

099d106

[backend/frontend] Change after rebase (#14585)

19d8f1e

richard-julien force-pushed the issue/14585_loop branch from 845696c to 19d8f1e Compare March 4, 2026 17:41

richard-julien requested a review from Copilot March 4, 2026 19:18

Copilot started reviewing on behalf of richard-julien March 4, 2026 19:18 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

richard-julien added 2 commits March 4, 2026 22:00

[backend/frontend] Change after review (#14585)

ec8f031

[backend/frontend] remove import (#14585)

86faf20

xfournet approved these changes Mar 4, 2026

View reviewed changes

[backend/frontend] Adjust try/catch (#14585)

186f19f

xfournet approved these changes Mar 4, 2026

View reviewed changes

xfournet merged commit 25629f9 into master Mar 4, 2026
36 checks passed

xfournet deleted the issue/14585_loop branch March 4, 2026 22:20

-          try {
-            const currentFileUri = currentFile.uri;
+          try {
+            // If the file already contains data (common in STIX 2.1 bundles), reuse it directly.
+            if (currentFile.data) {
+              copiedFiles.push({ ...currentFile });
+              // eslint-disable-next-line no-continue
+              continue;
+            }
+            const currentFileUri = currentFile.uri;
+            // In some STIX 2.1 conversions, uri can be 'unknown' or missing; avoid bogus storage lookups.
+            if (!currentFileUri || currentFileUri === 'unknown') {
+              logApp.warn('Cant copy file from main element to the container: invalid or missing uri', {
+                name: currentFile.name,
+                uri: currentFileUri,
+              });
+              // eslint-disable-next-line no-continue
+              continue;
+            }
+            if (!currentFileUri.startsWith('/storage/get/')) {
+              logApp.warn('Cant copy file from main element to the container: unexpected uri format', {
+                name: currentFile.name,
+                uri: currentFileUri,
+              });
+              // eslint-disable-next-line no-continue
+              continue;
+            }

Conversation

richard-julien commented Mar 3, 2026

Uh oh!

codecov bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aHenryJard Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xfournet Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

aHenryJard left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

xfournet Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Mar 3, 2026 •

edited

Loading