chore: restrict writing to certain object keys (#176)

- Block 'prototype', '__proto__'& 'constructor'
- Updated 'addBuildPhase' to ensure buildPhaseType does not accept restricted key.
- Updated 'addTargetAttribute' to ensure target.uuid does not accept restricted key.
- Updated 'pbxCreateGroupWithType' to ensure groupType does not accept restricted key.
- Updated tests for 'addBuildPhase' & 'addTargetAttribute'
- Added test case for 'pbxCreateGroupWithType'
- Small test update to correct spelling of build phase type 'PBXFrameworksBuildPhase'

Author: erisu

lib/pbxProject.js

@@ -80,6 +80,15 @@ const COMMENT_KEY = /_comment$/;
 
 // MARK: End of Typings
 
+// MARK: Start of Internals
+
+/**
+ * A list of restricted object keys.
+ */
+const RESTRICTED_OBJECT_KEYS = new Set(['__proto__', 'prototype', 'constructor']);
+
+// MARK: End of Internals
+
 function PBXProject (filename) {
     if (!(this instanceof PBXProject)) { return new PBXProject(filename); }
 
@@ -921,6 +930,13 @@ PBXProject.prototype.addTargetDependency = function (target, dependencyTargets)
  * @returns {AddBuildPhaseResults} object containing the build phase & uuid
  */
 PBXProject.prototype.addBuildPhase = function (filePathsArray, buildPhaseType, comment, target, optionsOrFolderType, subfolderPath) {
+    // Xcode may introduce new build phase types so the buildPhaseType
+    // param will not be validated against a fixed set of values.
+    // Instead, it will reject object keys that shouldn't be writable.
+    if (RESTRICTED_OBJECT_KEYS.has(buildPhaseType)) {
+        throw new Error(`"${buildPhaseType}" is restricted and cannot be used as a build phase type.`);
+    }
+
     const fileReferenceSection = this.pbxFileReferenceSection();
     const buildFileSection = this.pbxBuildFileSection();
     const buildPhaseUuid = this.generateUuid();
@@ -1918,6 +1934,11 @@ PBXProject.prototype.pbxCreateGroupWithType = function (name, pathName, groupTyp
     // Create comment
     const commendId = key + '_comment';
 
+    // Reject object keys that shouldn't be writable.
+    if (RESTRICTED_OBJECT_KEYS.has(groupType)) {
+        throw new Error(`"${groupType}" is restricted and cannot be used as a group type.`);
+    }
+
     // add obj and commentObj to groups;
     let groups = this.hash.project.objects[groupType];
     if (!groups) {
@@ -2212,6 +2233,12 @@ PBXProject.prototype.addTargetAttribute = function (prop, value, target) {
         attributes.TargetAttributes = {};
     }
     target = target || this.getFirstTarget();
+
+    // Reject object keys that shouldn't be writable.
+    if (RESTRICTED_OBJECT_KEYS.has(target.uuid)) {
+        throw new Error(`"${target.uuid}" is restricted and cannot be used as a uuid.`);
+    }
+
     if (attributes.TargetAttributes[target.uuid] === undefined) {
         attributes.TargetAttributes[target.uuid] = {};
     }

test/addBuildPhase.js

@@ -219,4 +219,22 @@ describe('addBuildPhase', () => {
         assert.equal(buildPhase.shellScript, '"test"');
         assert.strictEqual(buildPhase.alwaysOutOfDate, undefined);
     });
+
+    it('should not throw error when a non-restricted value is used for build phase type.', () => {
+        assert.doesNotThrow(
+            () => {
+                proj.addBuildPhase(['MyAssets.xcassets'], 'FooBar', 'Resources');
+            },
+            /Error: "FooBar" is restricted and cannot be used as a build phase type\./
+        );
+    });
+
+    it('should throw error when a restricted value was passed in for a build phase type.', () => {
+        assert.throws(
+            () => {
+                proj.addBuildPhase(['MyAssets.xcassets'], 'prototype', 'Resources');
+            },
+            /Error: "prototype" is restricted and cannot be used as a build phase type\./
+        );
+    });
 });

test/addTarget.js

@@ -169,7 +169,7 @@ describe('addTarget', () => {
         const resourceFile = proj.addResourceFile('assets.bundle', options);
         const resourcePhase = proj.addBuildPhase([], 'PBXResourcesBuildPhase', 'Resources', target.uuid);
         const frameworkFile = proj.addFramework('libsqlite3.dylib', options);
-        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworkBuildPhase', 'Frameworks', target.uuid);
+        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworksBuildPhase', 'Frameworks', target.uuid);
         const headerFile = proj.addHeaderFile('file.h', options);
 
         assert.ok(sourcePhase);

test/addWatch2App.js

@@ -84,7 +84,7 @@ describe('addWatchApp', () => {
         const resourceFile = proj.addResourceFile('assets.bundle', options);
         const resourcePhase = proj.addBuildPhase([], 'PBXResourcesBuildPhase', 'Resources', target.uuid);
         const frameworkFile = proj.addFramework('libsqlite3.dylib', options);
-        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworkBuildPhase', 'Frameworks', target.uuid);
+        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworksBuildPhase', 'Frameworks', target.uuid);
         const headerFile = proj.addHeaderFile('file.h', options);
 
         assert.ok(sourcePhase);

test/addWatch2Extension.js

@@ -64,7 +64,7 @@ describe('addWatchExtension', () => {
         const resourceFile = proj.addResourceFile('assets.bundle', options);
         const resourcePhase = proj.addBuildPhase([], 'PBXResourcesBuildPhase', 'Resources', target.uuid);
         const frameworkFile = proj.addFramework('libsqlite3.dylib', options);
-        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworkBuildPhase', 'Frameworks', target.uuid);
+        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworksBuildPhase', 'Frameworks', target.uuid);
         const headerFile = proj.addHeaderFile('file.h', options);
 
         assert.ok(sourcePhase);

test/addWatchApp.js

@@ -84,7 +84,7 @@ describe('addWatchApp', () => {
         const resourceFile = proj.addResourceFile('assets.bundle', options);
         const resourcePhase = proj.addBuildPhase([], 'PBXResourcesBuildPhase', 'Resources', target.uuid);
         const frameworkFile = proj.addFramework('libsqlite3.dylib', options);
-        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworkBuildPhase', 'Frameworks', target.uuid);
+        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworksBuildPhase', 'Frameworks', target.uuid);
         const headerFile = proj.addHeaderFile('file.h', options);
 
         assert.ok(sourcePhase);

test/addWatchExtension.js

@@ -63,7 +63,7 @@ describe('addWatchExtension', () => {
         const resourceFile = proj.addResourceFile('assets.bundle', options);
         const resourcePhase = proj.addBuildPhase([], 'PBXResourcesBuildPhase', 'Resources', target.uuid);
         const frameworkFile = proj.addFramework('libsqlite3.dylib', options);
-        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworkBuildPhase', 'Frameworks', target.uuid);
+        const frameworkPhase = proj.addBuildPhase([], 'PBXFrameworksBuildPhase', 'Frameworks', target.uuid);
         const headerFile = proj.addHeaderFile('file.h', options);
 
         assert.ok(sourcePhase);

test/group.js

@@ -434,5 +434,23 @@ describe('group', () => {
             output = project.writeSync();
             assert.equal(output.match(/ProvisioningStyle\s*=\s*Manual/g), null);
         });
+
+        it('should not throw error when a non-restricted value is used for a target.uuid.', () => {
+            assert.doesNotThrow(
+                () => {
+                    project.addTargetAttribute('ProvisioningStyle', 'Manual', { uuid: 'FooBar' });
+                },
+                /Error: "FooBar" is restricted and cannot be used as a uuid\./
+            );
+        });
+
+        it('should throw error when a restricted value was passed in for a target.uuid.', () => {
+            assert.throws(
+                () => {
+                    project.addTargetAttribute('ProvisioningStyle', 'Manual', { uuid: 'prototype' });
+                },
+                /Error: "prototype" is restricted and cannot be used as a uuid\./
+            );
+        });
     });
 });

test/pbxCreateGroupWithType.js

@@ -0,0 +1,49 @@
+/*
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+*/
+
+const { describe, it, beforeEach } = require('node:test');
+const assert = require('node:assert');
+
+const PBXProject = require('../lib/pbxProject');
+let project;
+
+describe('pbxCreateGroupWithType', () => {
+    beforeEach(() => {
+        project = new PBXProject('test/parser/projects/group.pbxproj');
+        project.parseSync();
+    });
+
+    it('should not throw error when a non-restricted value is used as a group type.', () => {
+        assert.doesNotThrow(
+            () => {
+                project.pbxCreateGroupWithType('name', '.', 'FooBar');
+            },
+            /Error: "FooBar" is restricted and cannot be used as a group type\./
+        );
+    });
+
+    it('should throw error when a restricted value was passed in as a group type.', () => {
+        assert.throws(
+            () => {
+                project.pbxCreateGroupWithType('name', '.', 'prototype');
+            },
+            /Error: "prototype" is restricted and cannot be used as a group type\./
+        );
+    });
+});