izar / pytm

A Pythonic framework for threat modeling
Other
861 stars 161 forks source link

Introducing EmptyElement to avoid showing "invalid" elements on diagrams #232

Open oleh-mykytyuk opened 4 months ago

oleh-mykytyuk commented 4 months ago

In the issue #222 was mentioned creation of the spurious "invalid" elements. Also, in the PR #223 tried to override the name of auto-generated elements.

This PR goal is to skip using "invalid" elements on DFD and SEQ diagrams. Such elements can be a problem when a lot of Finding used for overriding findings (add additional information to the threats). This approach is mentioned in the official documentation (README file), but leads to problems on the diagrams.

What is done:

oleh-mykytyuk commented 4 months ago

Included this situation in tests

izar commented 4 months ago

hey, thanks for the PR. I am a bit full this week but will be taking a look asap.

oleh-mykytyuk commented 2 months ago

hey, thanks for the PR. I am a bit full this week but will be taking a look asap.

Greetings! Sorry for disturbing, but how about this PR?

izar commented 2 months ago

sorry, still prioritizing work, but i have not forgotten this!

neilpvirtualitics commented 1 month ago

Hey, thanks for working on this! I am using pytm, and just noticed this 'elementinvalid" issue on my diagram, when using findings.

I've tried a couple of workarounds, but I still have two problems (with 1.3.1), the elementinvalid issue, and the text of my "response" member that I add to my Findings, not appearing in the report. I added a response field to my template like this:

<details>
  <summary>   {{{{item.id}}}}  --  {{{{item.threat_id}}}}   --   {{{{item.description}}}}</summary>
  <h6> Targeted Element </h6>
  <p> {{{{item.target}}}} </p>
  <h6> Severity </h6>
  <p>{{{{item.severity}}}}</p>
  <h6>Example Instances</h6>
  <p>{{{{item.example}}}}</p>
  <h6>Mitigations</h6>
  <p>{{{{item.mitigations}}}}</p>
  <h6>References</h6>
  <p>{{{{item.references}}}}</p>
  <h6>Response</h6>
  <p>{{{{item.response}}}}</p>
  &emsp;
</details>

but in the html, that item.response is empty, even though I supply a response="""To Mitigate: foo""", in my Finding().

One of the workarounds I thought I'd try for the invalid element issue, is to redirect the model.py --dfd-colormap output to jq to try to use jq to edit-out the elementinvalid elements, but this output is not json. It's close to json, but jq won't deal with it.

izar commented 3 weeks ago

ho @oleh-mykytiuk will you be able to address @neilpvirtualitics comment or should i review/merge as is ?

oleh-mykytiuk commented 3 weeks ago

Hey, thanks for working on this! I am using pytm, and just noticed this 'elementinvalid" issue on my diagram, when using findings.

I've tried a couple of workarounds, but I still have two problems (with 1.3.1), the elementinvalid issue, and the text of my "response" member that I add to my Findings, not appearing in the report. I added a response field to my template like this:

<details>
  <summary>   {{{{item.id}}}}  --  {{{{item.threat_id}}}}   --   {{{{item.description}}}}</summary>
  <h6> Targeted Element </h6>
  <p> {{{{item.target}}}} </p>
  <h6> Severity </h6>
  <p>{{{{item.severity}}}}</p>
  <h6>Example Instances</h6>
  <p>{{{{item.example}}}}</p>
  <h6>Mitigations</h6>
  <p>{{{{item.mitigations}}}}</p>
  <h6>References</h6>
  <p>{{{{item.references}}}}</p>
  <h6>Response</h6>
  <p>{{{{item.response}}}}</p>
  &emsp;
</details>

but in the html, that item.response is empty, even though I supply a response="""To Mitigate: foo""", in my Finding().

One of the workarounds I thought I'd try for the invalid element issue, is to redirect the model.py --dfd-colormap output to jq to try to use jq to edit-out the elementinvalid elements, but this output is not json. It's close to json, but jq won't deal with it.

Hi! This is another issue. I saw the same. It can be overridden by using the property example in the template and Finding. this is because not all fields are used during report generation. e.g.:

fundings.py

finding_DO01_api_gw_used = Finding(
    threat_id="DO01",
    example="Optional API Gateway can be used to check and limit requests",
)

template.md:

## Findings

{elements:repeat:{{item.findings:if:

### {{item.name}}
{{item.findings:repeat:
---

{{{{item.description}}}}

| # | ThreatID | Severity               | Description              |Status                |
|:-:|:--------:|------------------------|:-------------------------|:----------------------|
| {{{{item.id}}}} | {{{{item.threat_id}}}} | {{{{item.severity}}}} | {{{{item.description}}}} | {{{{item.example}}}} |

**Mitigations**: {{{{item.mitigations}}}}

**References**: {{{{item.references}}}}

}}}}}

I may open another PR to fix this. This PR should not affect other issues or problems.

oleh-mykytyuk commented 3 weeks ago

ho @oleh-mykytiuk will you be able to address @neilpvirtualitics comment or should i review/merge as is ?

Hi @izar ! I replied ;) I think that this is another issue and should be fixed by another PR.