Merge pull request #9823 from kajalpareek-lab/compliance-downgrade

block COMPLIANCE to GOVERNANCE retention mode downgrade

Author: kajalpareek-lab

src/server/object_services/object_server.js

@@ -332,52 +332,68 @@ async function get_object_retention(req) {
     };
 }
 /**
- *
  * put_object_retention
- *
  */
 async function put_object_retention(req) {
     dbg.log1('put_object_retention:', req.rpc_params);
-
     throw_if_maintenance(req);
     load_bucket(req);
     const obj = await find_object_md(req);
     const info = get_object_info(obj, { role: req.role });
-
     if (req.role !== 'admin') {
         throw new RpcError('UNAUTHORIZED');
     }
     if (!req.bucket.object_lock_configuration || req.bucket.object_lock_configuration.object_lock_enabled !== 'Enabled') {
         throw new RpcError('INVALID_REQUEST');
     }
-    if (info.lock_settings && info.lock_settings.retention &&
-        (new Date(req.rpc_params.retention.retain_until_date) < new Date(info.lock_settings.retention.retain_until_date) ||
-            !req.rpc_params.retention)) {
-
-        if ((info.lock_settings.retention.mode === 'GOVERNANCE' && (!req.rpc_params.bypass_governance || req.role !== 'admin')) ||
-            info.lock_settings.retention.mode === 'COMPLIANCE') {
-            dbg.error('put object retention failed due object retention mode', obj);
-            throw new RpcError('OBJECT_LOCKED',
-                'Access Denied because object protected by object lock.');
-        }
-    }
-    let legal_hold;
-    if (info.lock_settings && info.lock_settings.legal_hold) {
-        legal_hold = { status: info.lock_settings.legal_hold.status };
-    }
+    const current_retention = info.lock_settings?.retention;
+    const new_retention = req.rpc_params.retention;
+    _throw_if_retention_update_forbidden({
+        key: obj.key,
+        obj_id: info.obj_id,
+        current_retention,
+        new_retention,
+        bypass_governance: Boolean(req.rpc_params.bypass_governance),
+    });
+    const legal_hold_status = info.lock_settings?.legal_hold?.status;
+    const legal_hold = legal_hold_status ? { status: legal_hold_status } : undefined;
     await MDStore.instance().update_object_by_id(
         obj._id, {
             lock_settings: {
                 retention: {
-                    mode: req.rpc_params.retention.mode,
-                    retain_until_date: req.rpc_params.retention.retain_until_date,
+                    mode: new_retention.mode,
+                    retain_until_date: new_retention.retain_until_date,
                 },
                 legal_hold,
             }
         }, undefined, undefined
     );
 }
 
+function _throw_if_retention_update_forbidden({ key, obj_id, current_retention, new_retention, bypass_governance } = {}) {
+    if (!current_retention) return;
+    const current_retain_until = new Date(current_retention.retain_until_date);
+    const log_ctx = { key, obj_id, current_retention, new_retention };
+    // Active COMPLIANCE cannot change mode, even when retain-until stays the same or extends.
+    if (new_retention &&
+        current_retention.mode === 'COMPLIANCE' &&
+        current_retain_until > new Date() &&
+        new_retention.mode !== 'COMPLIANCE') {
+        dbg.error('put object retention failed: cannot change active COMPLIANCE mode', log_ctx);
+        throw new RpcError('OBJECT_LOCKED',
+            'Access Denied because object protected by object lock.');
+    }
+    const date_shortened = !new_retention ||
+        new Date(new_retention.retain_until_date) < current_retain_until;
+    if (!date_shortened) return;
+    if ((current_retention.mode === 'GOVERNANCE' && !bypass_governance) ||
+        current_retention.mode === 'COMPLIANCE') {
+        dbg.error('put object retention failed due object retention mode', log_ctx);
+        throw new RpcError('OBJECT_LOCKED',
+            'Access Denied because object protected by object lock.');
+    }
+}
+
 const ZERO_SIZE_ETAG = crypto.createHash('md5').digest('hex');
 
 /**

src/test/integration_tests/api/s3/test_s3_worm.js

@@ -745,20 +745,58 @@ mocha.describe('s3 worm', function() {
         });
 
         mocha.it('should fail to change compliance mode to governance', async function() {
-            const futureDate = new Date();
-            futureDate.setDate(futureDate.getDate() + 30);
+            // Previous test extended COMPLIANCE retain-until to +60 days.
+            // Use +90 here (after that date) so this assert fails on mode change
+            // (COMPLIANCE -> GOVERNANCE), not on "cannot shorten COMPLIANCE".
+            const longer_retain_until_date = new Date();
+            longer_retain_until_date.setDate(longer_retain_until_date.getDate() + 90);
 
             await assert_throws_async(s3_owner.putObjectRetention({
                 Bucket: BKT1,
                 Key: COMPLIANCE_KEY,
                 VersionId: compliance_version_id,
                 Retention: {
                     Mode: 'GOVERNANCE',
-                    RetainUntilDate: futureDate
+                    RetainUntilDate: longer_retain_until_date
                 },
                 BypassGovernanceRetention: true
             }), 'AccessDenied', 'Access Denied because object protected by object lock.');
         });
+
+        mocha.it('should allow changing governance mode to compliance', async function() {
+            const GOVERNANCE_KEY = 'governance-to-compliance-test';
+            const retain_until = new Date();
+            retain_until.setDate(retain_until.getDate() + 30);
+
+            const put_res = await s3_owner.putObject({
+                Bucket: BKT1,
+                Key: GOVERNANCE_KEY,
+                Body: file_body,
+                ContentType: 'text/plain',
+                ObjectLockMode: 'GOVERNANCE',
+                ObjectLockRetainUntilDate: retain_until,
+            });
+
+            const res = await s3_owner.putObjectRetention({
+                Bucket: BKT1,
+                Key: GOVERNANCE_KEY,
+                VersionId: put_res.VersionId,
+                Retention: {
+                    Mode: 'COMPLIANCE',
+                    RetainUntilDate: retain_until,
+                },
+                BypassGovernanceRetention: true,
+            });
+            delete res.$metadata;
+            assert.deepEqual(res, {});
+
+            const conf = await s3_owner.getObjectRetention({
+                Bucket: BKT1,
+                Key: GOVERNANCE_KEY,
+                VersionId: put_res.VersionId,
+            });
+            assert.equal(conf.Retention.Mode, 'COMPLIANCE');
+        });
     });
 
     mocha.describe('legal hold and retention independence', function() {

src/test/unit_tests/internal/test_object_lock_retention.test.js

@@ -0,0 +1,181 @@
+/* Copyright (C) 2026 NooBaa */
+
+'use strict';
+
+const { VERSIONING } = require('../../../common/constants').S3;
+const SensitiveString = require('../../../util/sensitive_string');
+const system_utils = require('../../../server/utils/system_utils');
+const { MDStore } = require('../../../server/object_services/md_store');
+const object_server = require('../../../server/object_services/object_server');
+
+describe('object_lock - put_object_retention', () => {
+    const system_store = require('../../../server/system_services/system_store').get_instance();
+
+    let mdstore_instance_stub;
+    let update_object_by_id_stub;
+    const bucket_id = 'bucket_id_retention';
+    const system_id = 'system_id_retention';
+
+    function make_obj({ mode, retain_until_date }) {
+        const retain = retain_until_date || new Date(Date.now() + 7 * 24 * 3600 * 1000);
+        return {
+            _id: {
+                toHexString: () => 'objidhex',
+                getTimestamp: () => new Date(),
+            },
+            system: system_id,
+            bucket: bucket_id,
+            key: 'locked-key',
+            size: 1,
+            lock_settings: {
+                retention: {
+                    mode,
+                    retain_until_date: retain,
+                },
+            },
+        };
+    }
+
+    function make_req({ mode, retain_until_date, bypass_governance }) {
+        return {
+            role: 'admin',
+            system: {
+                _id: system_id,
+                buckets_by_name: {
+                    'test-bucket': {
+                        _id: bucket_id,
+                        name: 'test-bucket',
+                        versioning: VERSIONING.ENABLED,
+                        object_lock_configuration: { object_lock_enabled: 'Enabled' },
+                    },
+                },
+            },
+            rpc_params: {
+                bucket: new SensitiveString('test-bucket'),
+                key: 'locked-key',
+                bypass_governance,
+                retention: {
+                    mode,
+                    retain_until_date,
+                },
+            },
+        };
+    }
+
+    beforeEach(() => {
+        mdstore_instance_stub = {
+            find_object_latest: jest.fn(),
+            update_object_by_id: jest.fn().mockResolvedValue(undefined),
+            get_object_version_id: jest.fn().mockReturnValue('v1'),
+            make_md_id: jest.fn(),
+        };
+        update_object_by_id_stub = mdstore_instance_stub.update_object_by_id;
+        jest.spyOn(MDStore, 'instance').mockReturnValue(mdstore_instance_stub);
+        jest.spyOn(system_utils, 'system_in_maintenance').mockReturnValue(false);
+        if (!system_store.data) {
+            system_store.data = {};
+        }
+        system_store.data.get_by_id = jest.fn().mockReturnValue({
+            name: new SensitiveString('test-bucket'),
+            versioning: VERSIONING.ENABLED,
+        });
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    test('rejects COMPLIANCE to GOVERNANCE downgrade with same retain date', async () => {
+        const retain_until = new Date(Date.now() + 15 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'COMPLIANCE', retain_until_date: retain_until })
+        );
+        const req = make_req({
+            mode: 'GOVERNANCE',
+            retain_until_date: retain_until,
+            bypass_governance: true,
+        });
+
+        await expect(object_server.put_object_retention(req))
+            .rejects.toMatchObject({ rpc_code: 'OBJECT_LOCKED' });
+        expect(update_object_by_id_stub).not.toHaveBeenCalled();
+    });
+
+    test('rejects COMPLIANCE to GOVERNANCE downgrade with longer retain date', async () => {
+        const current_until = new Date(Date.now() + 15 * 24 * 3600 * 1000);
+        const longer_until = new Date(Date.now() + 90 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'COMPLIANCE', retain_until_date: current_until })
+        );
+        const req = make_req({
+            mode: 'GOVERNANCE',
+            retain_until_date: longer_until,
+            bypass_governance: true,
+        });
+
+        await expect(object_server.put_object_retention(req))
+            .rejects.toMatchObject({ rpc_code: 'OBJECT_LOCKED' });
+        expect(update_object_by_id_stub).not.toHaveBeenCalled();
+    });
+
+    test('allows GOVERNANCE to COMPLIANCE upgrade with same retain date', async () => {
+        const retain_until = new Date(Date.now() + 15 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'GOVERNANCE', retain_until_date: retain_until })
+        );
+        const req = make_req({
+            mode: 'COMPLIANCE',
+            retain_until_date: retain_until,
+        });
+
+        await expect(object_server.put_object_retention(req)).resolves.toBeUndefined();
+        expect(update_object_by_id_stub).toHaveBeenCalled();
+    });
+
+    test('allows extending COMPLIANCE retention while keeping COMPLIANCE mode', async () => {
+        const current_until = new Date(Date.now() + 15 * 24 * 3600 * 1000);
+        const longer_until = new Date(Date.now() + 60 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'COMPLIANCE', retain_until_date: current_until })
+        );
+        const req = make_req({
+            mode: 'COMPLIANCE',
+            retain_until_date: longer_until,
+        });
+
+        await expect(object_server.put_object_retention(req)).resolves.toBeUndefined();
+        expect(update_object_by_id_stub).toHaveBeenCalled();
+    });
+
+    test('allows setting GOVERNANCE after COMPLIANCE retention has expired', async () => {
+        const expired_until = new Date(Date.now() - 24 * 3600 * 1000);
+        const new_until = new Date(Date.now() + 7 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'COMPLIANCE', retain_until_date: expired_until })
+        );
+        const req = make_req({
+            mode: 'GOVERNANCE',
+            retain_until_date: new_until,
+        });
+
+        await expect(object_server.put_object_retention(req)).resolves.toBeUndefined();
+        expect(update_object_by_id_stub).toHaveBeenCalled();
+    });
+
+    test('rejects shortening active COMPLIANCE retention', async () => {
+        const current_until = new Date(Date.now() + 15 * 24 * 3600 * 1000);
+        const shorter_until = new Date(Date.now() + 1 * 24 * 3600 * 1000);
+        mdstore_instance_stub.find_object_latest.mockResolvedValue(
+            make_obj({ mode: 'COMPLIANCE', retain_until_date: current_until })
+        );
+        const req = make_req({
+            mode: 'COMPLIANCE',
+            retain_until_date: shorter_until,
+            bypass_governance: true,
+        });
+
+        await expect(object_server.put_object_retention(req))
+            .rejects.toMatchObject({ rpc_code: 'OBJECT_LOCKED' });
+        expect(update_object_by_id_stub).not.toHaveBeenCalled();
+    });
+});