fix(file-manager): authorize org-level file access by organization_id#4192
Open
gorkem-bwl wants to merge 2 commits into
Open
fix(file-manager): authorize org-level file access by organization_id#4192gorkem-bwl wants to merge 2 commits into
gorkem-bwl wants to merge 2 commits into
Conversation
Downloading, deleting, or viewing metadata for an organization-level file (project_id IS NULL) returned 403 Forbidden for everyone, including org admins. The access check compared file.org_id against the caller's org. org_id is a legacy column that is NULL for files created by newer flows (AI Trust Center, evidence uploads), so Number(null) = 0 never matched the real org id and every such request was denied. The canonical tenant column is organization_id, which getFileById already scopes the lookup by — so this re-check only needed to read the right column. ## Changes - fileManager.ctrl: all five org-file checks (downloadFile, removeFile, getFileMetadata, and two more) now compare organization_id, not org_id. - file.model: add the organization_id column to FileModel and the File interface. It was unmapped, so mapToModel silently dropped the real value. - file.repository: add organization_id to OrganizationFileMetadata. - tests: mock files now carry organization_id; the org-mismatch cases assert on organization_id. 72/72 pass. ## Notes - Tenant isolation was never actually weakened: getFileById filters WHERE organization_id = :organizationId, so a cross-org file returns 404 before this check. The re-check is belt-and-suspenders. - A separate, pre-existing bug (file content stored as a JSON-serialized buffer, making downloads open as damaged) is tracked separately and not addressed here.
Code-review catch on PR #4192: getFileMetadata regressed because it sources its row from getFileWithMetadata, whose explicit SELECT list projected f.org_id but not f.organization_id. The new access check Number(file.organization_id) !== orgId then read undefined -> NaN, which never equals orgId, so metadata for every org-level file 403'd — the exact bug this PR set out to fix, reintroduced on the metadata endpoint. The other handlers were unaffected because they use getFileById (SELECT *). - Add f.organization_id to the getFileWithMetadata SELECT list (WHERE already scoped by it). - Add a repository unit test asserting the column is in the SELECT projection (scoped to SELECT..FROM, since organization_id also appears in JOIN/WHERE). The controller test could not catch this — it mocks the repository, so the real SQL projection was never exercised. Verified the guard fails when the column is dropped.
MuhammadKhalilzadeh
approved these changes
Jun 29, 2026
MuhammadKhalilzadeh
left a comment
MuhammadKhalilzadeh
left a comment
Collaborator
There was a problem hiding this comment.
Evidences work fine now, but for AI T0rust Center, we're getting the followings on setting a logo:
2026-06-29T17:41:23.123Z [debug]: 🖼️ Uploading company logo
2026-06-29T17:41:23.171Z [info]: 124, 2026-06-29T17:41:23.171Z, error, failed to upload company logo, uploadCompanyLogo, aiTrustCentre.ctrl.ts
2026-06-29T17:41:40.048Z [info]: 125, 2026-06-29T17:41:40.048Z, processing, uploading company logo, uploadCompanyLogo, aiTrustCentre.ctrl.ts
2026-06-29T17:41:40.049Z [debug]: 🖼️ Uploading company logo
2026-06-29T17:41:40.175Z [info]: 126, 2026-06-29T17:41:40.175Z, error, failed to upload company logo, uploadCompanyLogo, aiTrustCentre.ctrl.ts
2026-06-29T17:41:54.358Z [info]: 127, 2026-06-29T17:41:54.358Z, processing, uploading company logo, uploadCompanyLogo, aiTrustCentre.ctrl.ts
2026-06-29T17:41:54.358Z [debug]: 🖼️ Uploading company logo
2026-06-29T17:41:54.394Z [info]: 128, 2026-06-29T17:41:54.394Z, error, failed to upload company logo, uploadCompanyLogo, aiTrustCentre.ctrl.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a 403 Forbidden when downloading (or deleting, or viewing metadata for) an organization-level file such as an evidence upload or an AI Trust Center file. The error affected everyone, including org admins.
Root cause
The access check for organization-level files (
project_id IS NULL) comparedfile.org_idagainst the caller's organization.org_idis a legacy column that is NULL for files created by newer flows (AI Trust Center, evidence uploads), soNumber(null) = 0never matched the real org id and every such request was denied. The canonical tenant column isorganization_id, whichgetFileByIdalready scopes the lookup by.In the DB, 41 of 42 files have
org_idNULL withorganization_idset, so the download path was broken for nearly every org-level file.Changes
fileManager.ctrl.ts— all five org-file access checks (downloadFile,removeFile,getFileMetadata, and two others) now compareorganization_idinstead of the legacyorg_id.file.model.ts— add theorganization_idcolumn toFileModeland theFileinterface. It was previously unmapped, somapToModelsilently dropped the real value.file.repository.ts— addorganization_idtoOrganizationFileMetadata.organization_id; the org-mismatch cases assert onorganization_id. 72/72 pass.Notes
getFileByIdfiltersWHERE organization_id = :organizationId, so a cross-org file returns 404 before this check runs. The re-check is belt-and-suspenders.