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

feat: enable cocoapods for --all-projects scanning #965

Merged
merged 1 commit into from
Jan 27, 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
20 changes: 14 additions & 6 deletions src/cli/commands/monitor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ async function monitor(...args0: MethodArgs): Promise<any> {
if (typeof args[args.length - 1] === 'object') {
options = (args.pop() as ArgsOptions) as MonitorOptions;
}

args = args.filter(Boolean);

// populate with default path (cwd) if no path given
Expand Down Expand Up @@ -85,9 +84,15 @@ async function monitor(...args0: MethodArgs): Promise<any> {
debug(`Processing ${path}...`);
try {
await validateMonitorPath(path, options.docker);
const guessedPackageManager = detect.detectPackageManager(path, options);
const analysisType =
(options.docker ? 'docker' : guessedPackageManager) || 'all';
let analysisType = 'all';
let packageManager;
if (options.allProjects) {
analysisType = 'all';
} else if (options.docker) {
analysisType = 'docker';
} else {
packageManager = detect.detectPackageManager(path, options);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only run this check if we are not in Docker or all-projects mode as it will fail when no supported manifests are found

}

const targetFile =
!options.scanAllUnmanaged && options.docker && !options.file // snyk monitor --docker (without --file)
Expand All @@ -100,7 +105,10 @@ async function monitor(...args0: MethodArgs): Promise<any> {
);

const analyzingDepsSpinnerLabel =
'Analyzing ' + analysisType + ' dependencies for ' + displayPath;
'Analyzing ' +
(packageManager ? packageManager : analysisType) +
' dependencies for ' +
displayPath;

await spinner(analyzingDepsSpinnerLabel);

Expand All @@ -115,7 +123,7 @@ async function monitor(...args0: MethodArgs): Promise<any> {
getDepsFromPlugin(path, {
...options,
path,
packageManager: guessedPackageManager,
packageManager,
}),
spinner.clear(analyzingDepsSpinnerLabel),
);
Expand Down
2 changes: 2 additions & 0 deletions src/lib/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export const AUTO_DETECTABLE_FILES: string[] = [
'packages.config',
'project.json',
'project.assets.json',
'Podfile',
'Podfile.lock',
];

// when file is specified with --file, we look it up here
Expand Down
6 changes: 6 additions & 0 deletions src/lib/find-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ function chooseBestManifest(
)[0];
return defaultManifest.path;
}
case 'cocoapods': {
const defaultManifest = files.filter((path) =>
['Podfile'].includes(path.base),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker, just a question why not 'Podfile' === path.base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no reason copy + 🍝, will update in the next PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it's only relevant for npm and yarn and we bring it for all of others. We will have refactor after we release everything I think. I bet we could simplify when more ecosystems will be supported with --all-projects

Copy link
Contributor Author

@lili2311 lili2311 Jan 27, 2020

Choose a reason for hiding this comment

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

It is relevant for any ecosystem where more than 1 manifest file can be detected in the same folder and we need to choose. Example: requiremens.txt or Pipfile yarn.lock or package-lock.json.
Technically we could only look for Podfile here and remove this entry, but there are other cocopoads manifests we may auto detect later on which are currently supported via --file

)[0];
return defaultManifest.path;
}
default: {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface MonitorOptions {
'print-deps'?: boolean;
'experimental-dep-graph'?: boolean;
scanAllUnmanaged?: boolean;
allProjects?: boolean;
// An experimental flag to allow monitoring of bigtrees (with degraded deps info and remediation advice).
'prune-repeated-subdependencies'?: boolean;
}
Expand Down
30 changes: 30 additions & 0 deletions test/acceptance/cli-monitor/cli-monitor.all-projects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,5 +269,35 @@ export const AllProjectsTests: AcceptanceTests = {
'sends version number',
);
},
'`monitor monorepo-with-nuget with Cocoapods --all-projects and without same meta`': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
const spyPlugin = sinon.spy(params.plugins, 'loadPlugin');
t.teardown(spyPlugin.restore);

await params.cli.monitor('monorepo-with-nuget/src/cocoapods-app', {
allProjects: true,
});
// Pop all calls to server and filter out calls to `featureFlag` endpoint
const [cocoapodsAll] = params.server
.popRequests(1)
.filter((req) => req.url.includes('/monitor/'));

// Cocoapods
await params.cli.monitor('monorepo-with-nuget/src/cocoapods-app', {
file: 'Podfile',
});
const [requestsCocoapods] = params.server
.popRequests(1)
.filter((req) => req.url.includes('/monitor/'));

t.deepEqual(
cocoapodsAll.body,
requestsCocoapods.body,
'Same body for --all-projects and --file=src/cocoapods-app/Podfile',
);
},
},
};
18 changes: 17 additions & 1 deletion test/acceptance/cli-test/cli-test.all-projects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,16 @@ export const AllProjectsTests: AcceptanceTests = {
allProjects: true,
detectionDepth: 4,
});
t.ok(
spyPlugin.withArgs('cocoapods').callCount,
1,
'calls cocoapods plugin',
);
t.ok(spyPlugin.withArgs('nuget').callCount, 2, 'calls nuget plugin');
t.ok(spyPlugin.withArgs('npm').calledOnce, 'calls npm plugin');
t.match(
res,
/Tested 3 projects, no vulnerable paths were found./,
/Tested 4 projects, no vulnerable paths were found./,
'Two projects tested',
);
t.match(
Expand All @@ -420,6 +425,17 @@ export const AllProjectsTests: AcceptanceTests = {
'Nuget project targetFile is as expected',
);
t.match(res, 'Package manager: nuget', 'Nuget package manager');

t.match(
res,
`Target file: src${path.sep}cocoapods-app${path.sep}Podfile`,
'Cocoapods project targetFile is as expected',
);
t.match(
res,
'Package manager: cocoapods',
'cocoapods package manager',
);
} catch (err) {
t.fail('expected to pass');
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
target 'SampleApp' do
platform :ios, '6.0'
pod 'Reachability', '3.1.0'
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
PODS:
- Reachability (3.1.0)

DEPENDENCIES:
- Reachability (= 3.1.0)

SPEC REPOS:
trunk:
- Reachability

SPEC CHECKSUMS:
Reachability: 3c8fe9643e52184d17f207e781cd84158da8c02b

PODFILE CHECKSUM: eef52b2296b88c87f94cf0f232f010176b9f11cd

This file was deleted.

This file was deleted.