Merge branch 'improvement/CLDSRV-650' into w/9.1/improvement/CLDSRV-650

francoisferrand · francoisferrand · commit 7472985fe525 · 2025-05-26T19:10:43.000+02:00
diff --git a/lib/Config.js b/lib/Config.js
@@ -872,14 +872,26 @@ class Config extends EventEmitter {
             }
         }
 
-        this.port = 8000;
         if (config.port !== undefined) {
             assert(Number.isInteger(config.port) && config.port > 0,
                 'bad config: port must be a positive integer');
-            this.port = config.port;
         }
 
+        if (config.internalPort !== undefined) {
+            assert(Number.isInteger(config.internalPort) && config.internalPort > 0,
+                'bad config: internalPort must be a positive integer');
+        }
+
+        this.port = config.port;
         this.listenOn = this._parseEndpoints(config.listenOn, 'listenOn');
+        this.internalPort = config.internalPort;
+        this.internalListenOn = this._parseEndpoints(config.internalListenOn, 'internalListenOn');
+
+        if (this.listenOn.length || (!this.internalPort && !this.internalListenOn.length)) {
+            // default to 8000 unless running in "internal" API mode only (i.e. internalPort/ListenOn)
+            // i.e. when either no port config is specified, or only listenOn is specified.
+            this.port ||= 8000;
+        }
 
         this.metricsPort = 8002;
         if (config.metricsPort !== undefined) {
diff --git a/lib/api/apiUtils/authorization/permissionChecks.js b/lib/api/apiUtils/authorization/permissionChecks.js
@@ -358,7 +358,7 @@ function processBucketPolicy(requestType, bucket, canonicalID, arn, bucketOwner,
     request, aclPermission, results, actionImplicitDenies) {
     const bucketPolicy = bucket.getBucketPolicy();
     let processedResult = results[requestType];
-    if (!bucketPolicy) {
+    if (!bucketPolicy || request?.bypassUserBucketPolicies) {
         processedResult = actionImplicitDenies[requestType] === false && aclPermission;
     } else {
         const bucketPolicyPermission = checkBucketPolicy(bucketPolicy, requestType, canonicalID, arn,
diff --git a/lib/server.js b/lib/server.js
@@ -26,7 +26,6 @@ const {
 
 const HttpAgent = require('agentkeepalive');
 const QuotaService = require('./utilization/instance');
-const routes = arsenal.s3routes.routes;
 const { parseLC, MultipleBackendGateway } = arsenal.storage.data;
 const websiteEndpoints = _config.websiteEndpoints;
 let client = dataWrapper.client;
@@ -62,9 +61,11 @@ class S3Server {
     /**
      * This represents our S3 connector.
      * @constructor
+     * @param {Object} config - Configuration object
      * @param {Worker} [worker=null] - Track the worker when using cluster
      */
-    constructor(worker) {
+    constructor(config, worker) {
+        this.config = config;
         this.worker = worker;
         this.cluster = true;
         this.servers = [];
@@ -93,11 +94,29 @@ class S3Server {
         this.started = false;
     }
 
+    /**
+     * Route requests on 'internal s3' port
+     * Same as routeRequest, but ignoring user's bucket policy. This should be used only for
+     * backbeat and other internal/system processes that must not be affected by user's bucket
+     * policy.
+     * Note that this is not a temporary measure: eventually this should be improved to better
+     * identify and isolate internal/system calls vs user calls, so that "system" bucket policies
+     * may be applied to system requests, and the existing "user" bucket policies apply only to
+     * user requests.
+     * @param {http.IncomingMessage} req - http request object
+     * @param {http.ServerResponse} res - http response object
+     * @returns {undefined}
+     */
+    internalRouteRequest(req, res) {
+        req.bypassUserBucketPolicies = true; // eslint-disable-line no-param-reassign
+        return this.routeRequest(req, res);
+    }
+
     /**
      * Route requests on 's3' port
      * @param {http.IncomingMessage} req - http request object
      * @param {http.ServerResponse} res - http response object
-     * @returns {void}
+     * @returns {undefined}
      */
     routeRequest(req, res) {
         monitoringClient.httpActiveRequests.inc();
@@ -144,21 +163,21 @@ class S3Server {
             dataRetrievalParams: {
                 client,
                 implName,
-                config: _config,
+                config: this.config,
                 kms,
                 metadata,
                 locStorageCheckFn: locationStorageCheck,
                 vault,
             },
         };
-        routes(req, res, params, logger, _config);
+        arsenal.s3routes.routes(req, res, params, logger, this.config);
     }
 
     /**
      * Route requests on 'admin' port
      * @param {http.IncomingMessage} req - http request object
      * @param {http.ServerResponse} res - http response object
-     * @returns {void}
+     * @returns {undefined}
      */
     routeAdminRequest(req, res) {
         // use proxied hostname if needed
@@ -205,16 +224,16 @@ class S3Server {
      * @param {http.RequestListener} listener - Callback to handle requests
      * @param {number} port - Port to listen on
      * @param {string | undefined} ipAddress - IpAddress to listen on
-     * @returns {void}
+     * @returns {undefined}
      */
     _startServer(listener, port, ipAddress) {
         // Todo: http.globalAgent.maxSockets, http.globalAgent.maxFreeSockets
         let server;
-        if (_config.https) {
+        if (this.config.https) {
             server = https.createServer({
-                cert: _config.https.cert,
-                key: _config.https.key,
-                ca: _config.https.ca,
+                cert: this.config.https.cert,
+                key: this.config.https.key,
+                ca: this.config.https.ca,
                 ciphers: arsenal.https.ciphers.ciphers,
                 dhparam: arsenal.https.dhparam.dhparam,
                 rejectUnauthorized: true,
@@ -305,21 +324,30 @@ class S3Server {
             }
 
             // Start API server(s)
-            if (_config.listenOn.length > 0) {
-                _config.listenOn.forEach(item => {
+            if (this.config.listenOn.length > 0) {
+                this.config.listenOn.forEach(item => {
                     this._startServer(this.routeRequest, item.port, item.ip);
                 });
-            } else {
-                this._startServer(this.routeRequest, _config.port);
+            } else if (this.config.port) {
+                this._startServer(this.routeRequest, this.config.port);
+            }
+
+            // Start internal API server(s)
+            if (this.config.internalListenOn.length > 0) {
+                this.config.internalListenOn.forEach(item => {
+                    this._startServer(this.internalRouteRequest, item.port, item.ip);
+                });
+            } else if (this.config.internalPort) {
+                this._startServer(this.internalRouteRequest, this.config.internalPort);
             }
 
             // Start metrics server(s)
-            if (_config.metricsListenOn.length > 0) {
-                _config.metricsListenOn.forEach(item => {
+            if (this.config.metricsListenOn.length > 0) {
+                this.config.metricsListenOn.forEach(item => {
                     this._startServer(this.routeAdminRequest, item.port, item.ip);
                 });
             } else {
-                this._startServer(this.routeAdminRequest, _config.metricsPort);
+                this._startServer(this.routeAdminRequest, this.config.metricsPort);
             }
 
             // Start quota service health checks
@@ -353,7 +381,7 @@ function main() {
     this.cluster = workers > 1;
     if (!this.cluster) {
         process.env.REPORT_TOKEN = _config.reportToken;
-        const server = new S3Server();
+        const server = new S3Server(_config);
         server.initiateStartup(logger.newRequestLogger());
     }
     if (this.cluster && cluster.isMaster) {
@@ -399,10 +427,11 @@ function main() {
         });
     }
     if (this.cluster && cluster.isWorker) {
-        const server = new S3Server(cluster.worker);
+        const server = new S3Server(_config, cluster.worker);
         server.initiateStartup(logger.newRequestLogger());
     }
     monitoringClient.collectDefaultMetrics({ timeout: 5000 });
 }
 
 module.exports = main;
+module.exports.S3Server = S3Server;
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@zenko/cloudserver",
-  "version": "9.0.11",
+  "version": "9.0.12",
   "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
   "main": "index.js",
   "engines": {
diff --git a/tests/unit/Config.js b/tests/unit/Config.js
@@ -12,6 +12,8 @@ const {
 const { ValidLifecycleRules: supportedLifecycleRules } = require('arsenal').models;
 
 describe('Config', () => {
+    const defaultConfig = JSON.parse(fs.readFileSync('config.json'), { encoding: 'utf-8' });
+
     const envToRestore = [];
     const setEnv = (key, value) => {
         if (key in process.env) {
@@ -446,7 +448,6 @@ describe('Config', () => {
     });
 
     describe('multiObjectDeleteEnableOptimizations', () => {
-        const defaultConfig = JSON.parse(fs.readFileSync('config.json'), { encoding: 'utf-8' });
         let sandbox;
         let readFileStub;
 
@@ -679,4 +680,134 @@ describe('Config', () => {
             assert.deepStrictEqual(parsedRules, rules);
         });
     });
+
+    describe('port and internalPort configuration', () => {
+        // same as `defaultConfig` (i.e. config.json) but without the port settings
+        const baseConfig = (() => {
+            const config = { ...defaultConfig };
+            delete config.port;
+            delete config.listenOn;
+            delete config.internalPort;
+            delete config.internalListenOn;
+            delete config.metricsPort;
+            delete config.metricsListenOn;
+            return config;
+        })();
+
+        let sandbox;
+        let readFileStub;
+
+        beforeEach(() => {
+            sandbox = sinon.createSandbox();
+            readFileStub = sandbox.stub(fs, 'readFileSync');
+            readFileStub.callThrough();
+        });
+
+        afterEach(() => {
+            sandbox.restore();
+        });
+
+        it('should throw an error for negative port number', () => {
+            const testConfig = { ...baseConfig, port: -1 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            assert.throws(() => new ConfigObject(), /bad config: port must be a positive integer/);
+        });
+
+        it('should throw an error for non-integer port number', () => {
+            const testConfig = { ...baseConfig, port: 8000.5 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            assert.throws(() => new ConfigObject(), /bad config: port must be a positive integer/);
+        });
+
+        it('should throw an error for negative internalPort number', () => {
+            const testConfig = { ...baseConfig, internalPort: -1 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            assert.throws(() => new ConfigObject(), /bad config: internalPort must be a positive integer/);
+        });
+
+        it('should throw an error for non-integer internalPort number', () => {
+            const testConfig = { ...baseConfig, internalPort: 9000.5 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            assert.throws(() => new ConfigObject(), /bad config: internalPort must be a positive integer/);
+        });
+
+        it('should accept a valid port number', () => {
+            const testConfig = { ...baseConfig, port: 8765 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, 8765);
+            assert.strictEqual(config.internalPort, undefined);
+        });
+
+        it('should accept a valid internalPort number', () => {
+            const testConfig = { ...baseConfig, internalPort: 9000 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, undefined);
+            assert.strictEqual(config.internalPort, 9000);
+        });
+
+        it('should accept both port and internalPort when provided', () => {
+            const testConfig = { ...baseConfig, port: 8000, internalPort: 9000 };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, 8000);
+            assert.strictEqual(config.internalPort, 9000);
+        });
+
+        it('should use default port 8000 if no port or internalPort specified', () => {
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(baseConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, 8000);
+            assert.strictEqual(config.internalPort, undefined);
+        });
+
+        it('should default port if listenOn is provided', () => {
+            const testConfig = {
+                ...baseConfig,
+                listenOn: ['127.0.0.1:9000'],
+            };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, 8000);
+            assert.strictEqual(config.internalPort, undefined);
+            assert.deepEqual(config.listenOn, [{ ip: '127.0.0.1', port: 9000 }]);
+        });
+
+        it('should not default port if internalListenOn is provided', () => {
+            const testConfig = {
+                ...baseConfig,
+                internalListenOn: ['127.0.0.1:9000'],
+            };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, undefined);
+            assert.strictEqual(config.internalPort, undefined);
+            assert.deepEqual(config.internalListenOn, [{ ip: '127.0.0.1', port: 9000 }]);
+        });
+
+        it('should properly handle listenOn and internalListenOn configuration', () => {
+            const testConfig = {
+                ...baseConfig,
+                listenOn: ['0.0.0.0:8000'],
+                internalListenOn: ['127.0.0.1:9000'],
+            };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
+
+            const config = new ConfigObject();
+            assert.strictEqual(config.port, 8000);
+            assert.deepEqual(config.listenOn, [{ ip: '0.0.0.0', port: 8000 }]);
+            assert.deepEqual(config.internalListenOn, [{ ip: '127.0.0.1', port: 9000 }]);
+        });
+    });
 });
diff --git a/tests/unit/api/bucketPolicyAuth.js b/tests/unit/api/bucketPolicyAuth.js
diff --git a/tests/unit/server.js b/tests/unit/server.js

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@zenko/cloudserver",`
`3`		`- "version": "9.0.11",`
	`3`	`+ "version": "9.0.12",`
`4`	`4`	`"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",`
`5`	`5`	`"main": "index.js",`
`6`	`6`	`"engines": {`