Repository: electron-userland/electron-forge
Pull request #341 (2017-09-27 21:37:55)

fix(generic): correct the getOwnPropertyDescriptor proxy hook to resp…
…ect current properties writabil

ISSUES CLOSED: #340 

* [x] I have read the [contribution documentation](https://github.com/electron-userland/electron-forge/blob/master/CONTRIBUTING.md) for this project.
* [x] I agree to follow the [code of conduct](https://github.com/electron/electron/blob/master/CODE_OF_CONDUCT.md) that this project follows, as appropriate.
* [x] The changes are appropriately documented (if applicable).
* [x] The changes have sufficient test coverage (if applicable).
* [x] The testsuite passes successfully on my local machine (if applicable).



Patch:
diff --git a/src/util/forge-config.js b/src/util/forge-config.js
index d607af51b..ad3354fbc 100644
--- a/src/util/forge-config.js
+++ b/src/util/forge-config.js
@@ -28,8 +28,10 @@ const proxify = (object, envPrefix) => {
     getOwnPropertyDescriptor(target, name) {
       const envValue = process.env[`${envPrefix}_${underscoreCase(name)}`];
       // eslint-disable-next-line no-prototype-builtins
-      if (target.hasOwnProperty(name) || envValue) {
-        return { configurable: true, enumerable: true };
+      if (target.hasOwnProperty(name)) {
+        return Object.getOwnPropertyDescriptor(target, name);
+      } else if (envValue) {
+        return { writable: true, enumerable: true, configurable: true, value: envValue };
       }
     },
   });


Test patch:
diff --git a/test/fast/forge-config_spec.js b/test/fast/forge-config_spec.js
index 9d5319972..1ced60fa4 100644
--- a/test/fast/forge-config_spec.js
+++ b/test/fast/forge-config_spec.js
@@ -37,6 +37,7 @@ describe('forge-config', () => {
       },
     }));
   });
+
   it('should allow access to built-ins of proxied objects', async () => {
     const conf = await findConfig(path.resolve(__dirname, '../fixture/dummy_js_conf'));
     expect(conf.electronPackagerConfig.baz.hasOwnProperty).to.be.a('function');
@@ -46,6 +47,19 @@ describe('forge-config', () => {
     delete process.env.ELECTRON_FORGE_S3_SECRET_ACCESS_KEY;
   });
 
+  it('should allow overwrite of properties in proxied objects', async () => {
+    const conf = await findConfig(path.resolve(__dirname, '../fixture/dummy_js_conf'));
+    expect(conf.electronPackagerConfig.baz.hasOwnProperty).to.be.a('function');
+    expect(() => { conf.electronPackagerConfig.baz = 'bar'; }).to.not.throw();
+    process.env.ELECTRON_FORGE_S3_SECRET_ACCESS_KEY = 'SecretyThing';
+
+    const descriptor = { writable: true, enumerable: true, configurable: true, value: 'SecretyThing' };
+    expect(Object.getOwnPropertyDescriptor(conf.s3, 'secretAccessKey')).to.be.deep.equal(descriptor);
+    expect(() => { conf.s3.secretAccessKey = 'bar'; }).to.not.throw();
+    expect(conf.s3.secretAccessKey).to.equal('bar');
+    delete process.env.ELECTRON_FORGE_S3_SECRET_ACCESS_KEY;
+  });
+
 
   it('should resolve the JS file exports in config.forge points to a JS file', async () => {
     expect(JSON.parse(JSON.stringify(await findConfig(path.resolve(__dirname, '../fixture/dummy_js_conf'))))).to.be.deep.equal(Object.assign({}, defaults, {