4 ms·

The big missing feature for these kinds of tools is a workflow and relationship for dev teams to mark findings. Marking them as "false positive" or "only appli

by tyingq 4y ago

The big missing feature for these kinds of tools is a workflow and relationship for dev teams to mark findings. Marking them as "false positive" or "only applies if these other conditions are true", or "yes, but we have a mitigation/exception". etc. A fast workflow that allows for less blockers, reduced noise and a focus on things that actually matter.

gmontard 4y ago
Workflow is coming with our Cloud offering, with all the cool integration you can think of as Jira or Slack. On the "marking" part, we have two options that will be available super soon: 1) Directly in the code, by adding a special comment that will ignore findings. 2) In the Cloud, an ignore action will forever park an issue, even if it changes line etc. (smart fingerprinting applied). We can't really have that in the OSS since it's state-less.
brightball 4y ago
Gitlab has a great security dashboard for this. It organizes the output of multiple tools in a place where you can discuss, triage, ignore or track an issue to resolve it. https://docs.gitlab.com/ee/user/application_security/security_dashboard/ https://docs.gitlab.com/ee/user/application_security/securit...
- gmontard 4y ago
  Also, super expensive, you need the $99 plan :) https://about.gitlab.com/pricing/ https://about.gitlab.com/pricing/ Integration with SCM is clearly a top priority for us, especially directly in PR. GitHub SARIF is a nice way to integrate third-party into their Dashboard, we're commited to it.
ecares 4y ago
Github actually has this feature (only for open source and enterprise IIRC) when there is a SARIF output
- gmontard 4y ago
  SARIF output is on our Roadmap btw! Github code scanning is not so great from what we've heard so far, but also it's very expensive, you need to be on the Enterprise plan...
spmurrayzzz 4y ago
Totally agree. I love the idea of SAST-in-CI, but I ran this on a handful of repos I manage (ranging from 40k-100k SLOC) and there were too many false positives for me want to add this as build-breaking criteria to our CI pipeline. Not unique at all to bearer in any way of course, as you point out, but still a real problem. I suppose an alternative would be to not have this be a zero-sum part of CI, but maybe as a qualitative summary that gets autogenerated as part of the PR / code review process. The noise issue is still a real one as people will eventually ignore the noisy summaries or filter/whitelist them into relative oblivion. I like the idea of "only applies if these other conditions are true". In all the false positives I encountered so far, if given the option I would be able to declaratively express when and when not to apply the rule. I'd even be ok with inline ignore comments to that end which, while not ideal, is something folks are already used to for other idioms like test coverage et al.
- cfabianski 4y ago
  I’ve introduced the `absence` trigger that does that If express is present but helmet is missing then break. Do you think that’d help achieve what you have in mind?
  - spmurrayzzz 4y ago
    I think the design flaw in most of the problematic rules was from too simple of regex matching. Looking for a string pattern should be a clue to do some deeper analysis (maybe verify via AST), not necessarily to flag the string alone as security failure.
    - didroe 4y ago
      The rules do work on the AST but the current cookie rule is not as advanced as it could/should be. For example, we really should treat encryption as sanitizing the value. We'll take another look at the rules with this in mind. If you are able to share the (rough) approach you take to build the cookie string it would help us to ensure we're covering the specific case(s) you have.
- 4y ago
eranation 4y ago
100% agree (shameless plug, the product we are working on for the last 1.5 years aims to solve exactly that… either via a PR bot, slack / teams etc). Ping me (see profile for details) if it’s interesting.

by tyingq 4y ago

gmontard 4y ago

Workflow is coming with our Cloud offering, with all the cool integration you can think of as Jira or Slack. On the "marking" part, we have two options that will be available super soon: 1) Directly in the code, by adding a special comment that will ignore findings. 2) In the Cloud, an ignore action will forever park an issue, even if it changes line etc. (smart fingerprinting applied). We can't really have that in the OSS since it's state-less.

brightball 4y ago

Gitlab has a great security dashboard for this. It organizes the output of multiple tools in a place where you can discuss, triage, ignore or track an issue to resolve it. https://docs.gitlab.com/ee/user/application_security/security_dashboard/ https://docs.gitlab.com/ee/user/application_security/securit...

gmontard 4y ago
Also, super expensive, you need the $99 plan :) https://about.gitlab.com/pricing/ https://about.gitlab.com/pricing/ Integration with SCM is clearly a top priority for us, especially directly in PR. GitHub SARIF is a nice way to integrate third-party into their Dashboard, we're commited to it.

ecares 4y ago

Github actually has this feature (only for open source and enterprise IIRC) when there is a SARIF output

gmontard 4y ago
SARIF output is on our Roadmap btw! Github code scanning is not so great from what we've heard so far, but also it's very expensive, you need to be on the Enterprise plan...

spmurrayzzz 4y ago

Totally agree. I love the idea of SAST-in-CI, but I ran this on a handful of repos I manage (ranging from 40k-100k SLOC) and there were too many false positives for me want to add this as build-breaking criteria to our CI pipeline. Not unique at all to bearer in any way of course, as you point out, but still a real problem. I suppose an alternative would be to not have this be a zero-sum part of CI, but maybe as a qualitative summary that gets autogenerated as part of the PR / code review process. The noise issue is still a real one as people will eventually ignore the noisy summaries or filter/whitelist them into relative oblivion. I like the idea of "only applies if these other conditions are true". In all the false positives I encountered so far, if given the option I would be able to declaratively express when and when not to apply the rule. I'd even be ok with inline ignore comments to that end which, while not ideal, is something folks are already used to for other idioms like test coverage et al.

cfabianski 4y ago
I’ve introduced the `absence` trigger that does that If express is present but helmet is missing then break. Do you think that’d help achieve what you have in mind?
- spmurrayzzz 4y ago
  I think the design flaw in most of the problematic rules was from too simple of regex matching. Looking for a string pattern should be a clue to do some deeper analysis (maybe verify via AST), not necessarily to flag the string alone as security failure.
  - didroe 4y ago
    The rules do work on the AST but the current cookie rule is not as advanced as it could/should be. For example, we really should treat encryption as sanitizing the value. We'll take another look at the rules with this in mind. If you are able to share the (rough) approach you take to build the cookie string it would help us to ensure we're covering the specific case(s) you have.
4y ago

eranation 4y ago

100% agree (shameless plug, the product we are working on for the last 1.5 years aims to solve exactly that… either via a PR bot, slack / teams etc). Ping me (see profile for details) if it’s interesting.