Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kernel): tarball unpacking does not behave like 'npm install' #1766

Merged
merged 3 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions packages/@jsii/kernel/lib/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,23 @@ export class Kernel {

// Create the install directory (there may be several path components for @scoped/packages)
fs.mkdirpSync(packageDir);
// untar the archive to its final location
tar.extract({
strict: true,
file: req.tarball,
cwd: packageDir,
strip: 1, // Removes the 'package/' path element from entries
sync: true,
});

// Force umask to have npm-install-like permissions
const originalUmask = process.umask(0o022);
try {
// untar the archive to its final location
tar.extract({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use the built-in support in the tar library.

One of these options should help:

  • portable Omit metadata that is system-specific: ctime, atime, uid, gid, uname, gname, dev, ino, and nlink. Note that mtime is still included, because this is necessary for other time-based operations. Additionally, mode is set to a "reasonable default" for most unix systems, based on a umask value of 0o22.
  • mode The mode to set on the created file archive

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel portable is the right one to use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those options are specific to the create operation and this is the extract operation.

The only "built-in" option available is the umask option to tar.extract, however that is "internal" (and not exposed in the d.ts which is why I decided against using that).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay sorry my mistake.

cwd: packageDir,
file: req.tarball,
strict: true,
strip: 1, // Removes the 'package/' path element from entries
sync: true,
unlink: true,
});
} finally {
// Reset umask to the initial value
process.umask(originalUmask);
}

// read .jsii metadata from the root of the package
const jsiiMetadataFile = path.join(packageDir, spec.SPEC_FILE_NAME);
Expand Down
22 changes: 22 additions & 0 deletions packages/@jsii/kernel/test/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,28 @@ defineTest.skip = function (
return defineTest(name, method, test.skip);
};

test('load preserves file permissions', async () => {
// Changing the umask to 077 (which would neutralize group/other permissions)
const originalUmask = process.umask(0o077);

try {
const kernel = await createCalculatorSandbox(
'load_preserves_file_permissions',
);

const result = kernel.sinvoke({
fqn: 'jsii-calc.UmaskCheck',
method: 'mode',
});
expect(result.result).toBe(0o644);

return closeRecording(kernel);
} finally {
// Restore the original umask
process.umask(originalUmask);
}
});

defineTest('stats() return sandbox statistics', (sandbox) => {
const stats = sandbox.stats({});
expect(stats.objectCount).toBe(0);
Expand Down
17 changes: 17 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2502,3 +2502,20 @@ export abstract class Isomorphism {
return this;
}
}

/**
* Checks the current file permissions are cool (no funky UMASK down-scoping happened)
*
* @see https://github.com/aws/jsii/issues/1765
*/
export class UmaskCheck {
/**
* This should return 0o644 (-rw-r--r--)
*/
public static mode(): number {
// The bit-masking is to remove the file type information from .mode
return fs.statSync(__filename).mode & 0o0777;
}

private constructor() {}
}
36 changes: 35 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -11869,6 +11869,40 @@
}
]
},
"jsii-calc.UmaskCheck": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/jsii/issues/1765",
"stability": "experimental",
"summary": "Checks the current file permissions are cool (no funky UMASK down-scoping happened)."
},
"fqn": "jsii-calc.UmaskCheck",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2511
},
"methods": [
{
"docs": {
"stability": "experimental",
"summary": "This should return 0o644 (-rw-r--r--)."
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2515
},
"name": "mode",
"returns": {
"type": {
"primitive": "number"
}
},
"static": true
}
],
"name": "UmaskCheck"
},
"jsii-calc.UnaryOperation": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -13162,5 +13196,5 @@
}
},
"version": "0.0.0",
"fingerprint": "miVjqwWxNOLMY7fR23c/SVvfiGqkgqLgG+18qp4f8MM="
"fingerprint": "/0S7DMfFaE1kfja4CpY5k0Gl6j6opo50SUPCiBgy3Zg="
}
Original file line number Diff line number Diff line change
Expand Up @@ -11869,6 +11869,40 @@
}
]
},
"jsii-calc.UmaskCheck": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/jsii/issues/1765",
"stability": "experimental",
"summary": "Checks the current file permissions are cool (no funky UMASK down-scoping happened)."
},
"fqn": "jsii-calc.UmaskCheck",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2511
},
"methods": [
{
"docs": {
"stability": "experimental",
"summary": "This should return 0o644 (-rw-r--r--)."
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2515
},
"name": "mode",
"returns": {
"type": {
"primitive": "number"
}
},
"static": true
}
],
"name": "UmaskCheck"
},
"jsii-calc.UnaryOperation": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -13162,5 +13196,5 @@
}
},
"version": "0.0.0",
"fingerprint": "miVjqwWxNOLMY7fR23c/SVvfiGqkgqLgG+18qp4f8MM="
"fingerprint": "/0S7DMfFaE1kfja4CpY5k0Gl6j6opo50SUPCiBgy3Zg="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using Amazon.JSII.Runtime.Deputy;

#pragma warning disable CS0672,CS0809,CS1591

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Checks the current file permissions are cool (no funky UMASK down-scoping happened).</summary>
/// <remarks>
/// <strong>Stability</strong>: Experimental
///
/// <strong>See</strong>: https://github.com/aws/jsii/issues/1765
/// </remarks>
[JsiiClass(nativeType: typeof(Amazon.JSII.Tests.CalculatorNamespace.UmaskCheck), fullyQualifiedName: "jsii-calc.UmaskCheck")]
public class UmaskCheck : DeputyBase
{
/// <summary>Used by jsii to construct an instance of this class from a Javascript-owned object reference</summary>
/// <param name="reference">The Javascript-owned object reference</param>
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
protected UmaskCheck(ByRefValue reference): base(reference)
{
}

/// <summary>Used by jsii to construct an instance of this class from DeputyProps</summary>
/// <param name="props">The deputy props</param>
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
protected UmaskCheck(DeputyProps props): base(props)
{
}

/// <summary>This should return 0o644 (-rw-r--r--).</summary>
/// <remarks>
/// <strong>Stability</strong>: Experimental
/// </remarks>
[JsiiMethod(name: "mode", returnsJson: "{\"type\":{\"primitive\":\"number\"}}")]
public static double Mode()
{
return InvokeStaticMethod<double>(typeof(Amazon.JSII.Tests.CalculatorNamespace.UmaskCheck), new System.Type[]{}, new object[]{});
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package software.amazon.jsii.tests.calculator;

/**
* Checks the current file permissions are cool (no funky UMASK down-scoping happened).
* <p>
* EXPERIMENTAL
* <p>
* @see https://github.com/aws/jsii/issues/1765
*/
@javax.annotation.Generated(value = "jsii-pacmak")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.$Module.class, fqn = "jsii-calc.UmaskCheck")
public class UmaskCheck extends software.amazon.jsii.JsiiObject {

protected UmaskCheck(final software.amazon.jsii.JsiiObjectRef objRef) {
super(objRef);
}

protected UmaskCheck(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) {
super(initializationMode);
}

/**
* This should return 0o644 (-rw-r--r--).
* <p>
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public static @org.jetbrains.annotations.NotNull java.lang.Number mode() {
return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.jsii.tests.calculator.UmaskCheck.class, "mode", java.lang.Number.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ jsii-calc.SupportsNiceJavaBuilderWithRequiredProps=software.amazon.jsii.tests.ca
jsii-calc.SyncVirtualMethods=software.amazon.jsii.tests.calculator.SyncVirtualMethods
jsii-calc.Thrower=software.amazon.jsii.tests.calculator.Thrower
jsii-calc.TopLevelStruct=software.amazon.jsii.tests.calculator.TopLevelStruct
jsii-calc.UmaskCheck=software.amazon.jsii.tests.calculator.UmaskCheck
jsii-calc.UnaryOperation=software.amazon.jsii.tests.calculator.UnaryOperation
jsii-calc.UnionProperties=software.amazon.jsii.tests.calculator.UnionProperties
jsii-calc.UpcasingReflectable=software.amazon.jsii.tests.calculator.UpcasingReflectable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8452,6 +8452,26 @@ def __repr__(self) -> str:
)


class UmaskCheck(metaclass=jsii.JSIIMeta, jsii_type="jsii-calc.UmaskCheck"):
"""Checks the current file permissions are cool (no funky UMASK down-scoping happened).

see
:see: https://github.com/aws/jsii/issues/1765
stability
:stability: experimental
"""

@jsii.member(jsii_name="mode")
@builtins.classmethod
def mode(cls) -> jsii.Number:
"""This should return 0o644 (-rw-r--r--).

stability
:stability: experimental
"""
return jsii.sinvoke(cls, "mode", [])


class UnaryOperation(
scope.jsii_calc_lib.Operation,
metaclass=jsii.JSIIAbstractClass,
Expand Down Expand Up @@ -9794,6 +9814,7 @@ def next(self) -> jsii.Number:
"SyncVirtualMethods",
"Thrower",
"TopLevelStruct",
"UmaskCheck",
"UnaryOperation",
"UnionProperties",
"UpcasingReflectable",
Expand Down
10 changes: 10 additions & 0 deletions packages/jsii-reflect/test/__snapshots__/jsii-tree.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,11 @@ exports[`jsii-tree --all 1`] = `
│ │ ├── <initializer>() initializer (experimental)
│ │ └─┬ throwError() method (experimental)
│ │ └── returns: void
│ ├─┬ class UmaskCheck (experimental)
│ │ └─┬ members
│ │ └─┬ static mode() method (experimental)
│ │ ├── static
│ │ └── returns: number
│ ├─┬ class UnaryOperation (experimental)
│ │ ├── base: Operation
│ │ └─┬ members
Expand Down Expand Up @@ -2799,6 +2804,7 @@ exports[`jsii-tree --inheritance 1`] = `
│ ├── class SupportsNiceJavaBuilderWithRequiredProps
│ ├── class SyncVirtualMethods
│ ├── class Thrower
│ ├── class UmaskCheck
│ ├─┬ class UnaryOperation
│ │ └── base: Operation
│ ├─┬ class UpcasingReflectable
Expand Down Expand Up @@ -3731,6 +3737,9 @@ exports[`jsii-tree --members 1`] = `
│ │ └─┬ members
│ │ ├── <initializer>() initializer
│ │ └── throwError() method
│ ├─┬ class UmaskCheck
│ │ └─┬ members
│ │ └── static mode() method
│ ├─┬ class UnaryOperation
│ │ └─┬ members
│ │ ├── <initializer>(operand) initializer
Expand Down Expand Up @@ -4301,6 +4310,7 @@ exports[`jsii-tree --types 1`] = `
│ ├── class SupportsNiceJavaBuilderWithRequiredProps
│ ├── class SyncVirtualMethods
│ ├── class Thrower
│ ├── class UmaskCheck
│ ├── class UnaryOperation
│ ├── class UpcasingReflectable
│ ├── class UseBundledDependency
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ Array [
"jsii-calc.SupportsNiceJavaBuilderWithRequiredProps",
"jsii-calc.SyncVirtualMethods",
"jsii-calc.Thrower",
"jsii-calc.UmaskCheck",
"jsii-calc.UnaryOperation",
"jsii-calc.UpcasingReflectable",
"jsii-calc.UseBundledDependency",
Expand Down