Skip to content

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

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.

Sign up for GitHub

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

Open
richard-julien wants to merge 4 commits into
base: master
Choose a base branch
from
+21 −26
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
236d0da
[backend/pycti] Prevent file upload looping (#14585)
richard-julien Mar 3, 2026
970ddc9
Update opencti-platform/opencti-graphql/src/modules/playbook/playbook…
richard-julien Mar 4, 2026
9072ce1
Update opencti-platform/opencti-graphql/src/modules/playbook/playbook…
richard-julien Mar 4, 2026
7a0d3d0
Update opencti-platform/opencti-graphql/src/modules/playbook/playbook…
richard-julien Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions client-python/pycti/utils/opencti_stix2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,13 +653,6 @@ def extract_embedded_relationships(
data = None
if "data" in file_obj:
data = base64.b64decode(file_obj["data"])
elif "uri" in file_obj:
file_url = self.opencti.api_url.replace(
"/graphql", file_obj["uri"]
)
data = self.opencti.fetch_opencti_file(
fetch_uri=file_url, binary=True, serialize=False
)
if data is not None:
files_to_upload.append(
self.opencti.file(
Expand Down Expand Up @@ -854,13 +847,6 @@ def extract_embedded_relationships(
data = None
if "data" in file_obj:
data = base64.b64decode(file_obj["data"])
elif "uri" in file_obj:
file_url = self.opencti.api_url.replace(
"/graphql", file_obj["uri"]
)
data = self.opencti.fetch_opencti_file(
fetch_uri=file_url, binary=True, serialize=False
)
if data is not None:
files_to_upload.append(
self.opencti.file(
Expand Down Expand Up @@ -1152,11 +1138,6 @@ def import_object(
data = None
if "data" in file_obj:
data = base64.b64decode(file_obj["data"])
elif "uri" in file_obj:
url = self.opencti.api_url.replace("/graphql", file_obj["uri"])
data = self.opencti.fetch_opencti_file(
fetch_uri=url, binary=True, serialize=False
)
if data is not None:
files_to_upload.append(
self.opencti.file(
Expand Down Expand Up @@ -1280,11 +1261,6 @@ def import_observable(
data = None
if "data" in file_obj:
data = base64.b64decode(file_obj["data"])
elif "uri" in file_obj:
url = self.opencti.api_url.replace("/graphql", file_obj["uri"])
data = self.opencti.fetch_opencti_file(
fetch_uri=url, binary=True, serialize=False
)
if data is not None:
files_to_upload.append(
self.opencti.file(
Expand Down
23 changes: 21 additions & 2 deletions opencti-platform/opencti-graphql/src/modules/playbook/playbook-components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ import { PLAYBOOK_DATA_STREAM_PIR } from './components/data-stream-pir-component
import { convertStoreToStix_2_1 } from '../../database/stix-2-1-converter';
import { ENTITY_TYPE_SECURITY_COVERAGE, INPUT_COVERED, type StixSecurityCoverage, type StoreEntitySecurityCoverage } from '../securityCoverage/securityCoverage-types';
import { pushAll } from '../../utils/arrayUtil';
import { getFileContent } from '../../database/raw-file-storage';

// region built in playbook components
interface LoggerConfiguration {
Expand Down Expand Up @@ -625,8 +626,26 @@ export const PLAYBOOK_CONTAINER_WRAPPER_COMPONENT: PlaybookComponent<ContainerWr
(<StixCaseIncident>container).extensions[STIX_EXT_OCTI].granted_refs = (<StixIncident>baseData).extensions[STIX_EXT_OCTI].granted_refs;
}
// Copy files from the main element to the container if requested
if (copyFiles && baseData.extensions[STIX_EXT_OCTI].files && baseData.extensions[STIX_EXT_OCTI].files.length > 0) {
container.extensions[STIX_EXT_OCTI].files = baseData.extensions[STIX_EXT_OCTI].files;
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;
Comment on lines +635 to +636
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copilot uses AI. Check for mistakes.
const fileId = currentFileUri.replace('/storage/get/', '');
const currentFileContent = await getFileContent(fileId, 'base64');
if (currentFileContent) {
copiedFiles.push({ ...currentFile, data: currentFileContent });
} else {
logApp.error("Can't copy file from main element to the container: empty content", { name: currentFile.name });
}
} catch (e) {
logApp.error("Can't copy file from main element to the container", { cause: e, name: currentFile.name });
}
}
container.extensions[STIX_EXT_OCTI].files = copiedFiles;
Comment on lines +629 to +648
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current loop drops a file entirely if it already has data (since its uri can be 'unknown') or if getFileContent fails, because only successfully fetched files are pushed to copiedFiles. To avoid unintended file removals, preserve existing currentFile.data when present and/or push the original currentFile into copiedFiles on failure (optionally marking it no_trigger_import).

Copilot uses AI. Check for mistakes.
Comment on lines +636 to +648
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
Comment on lines +629 to +648
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
}
if (STIX_DOMAIN_OBJECT_CONTAINER_CASES.includes(container_type) && caseTemplates.length > 0) {
const tasks = await createTaskFromCaseTemplates(caseTemplates, (container as StixContainer));
Expand Down