Add manual review page in External Security Reviews framework.

[the-caliber]

Changes:
Hi, as discussed previously on TG, i have created a PR, I created another page as the mental model was more inclined towards the approach for “manual review”.

Please let me know your thoughts on this @PatrickAlphaC
If this dedicated page makes sense And if any changes are needed!

Thank you!

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
frameworks	Ready	Preview	Comment	Aug 28, 2025 2:44pm

[PatrickAlphaC]

Hmm... This is a bit of a re-skin of what is in external-security-reviews. I think it would be better to have one canonical place for this type of review, and we can link to it, that way we don't have to update both folders/files.

Also, the formatting is very off in your markdown, could you please follow the new docs from this PR to format your markdown?

#171

[the-caliber]

Hey there!

Yes, i noticed the external-security-reviews is there. was wondering where to add this info/page(manual review).

1. Do you want me to add manual-review.md to external-security-reviews/ folder ? so that it can be linked to the Manual Review: in the src/external-security-reviews/smart-contracts/README.md ? Can you expand more on it :)

2. Also could you give more idea about the reason behind keeping external-security-reviews section different ?
   Maybe you already gave it here:

I think it would be better to have one canonical place for this type of review, and we can link to it

3. About formatting , Not sure why this seems off (at least when not rendered). But sure, i will look into your recommendation about feat: Devcontainer & linting docs #171

Thanks!

[mattaereal]

Note that the PR you mentioned has been closed and moved in favor of #184

[the-caliber]

Yep! noticed its now 184.

---

Just waiting for @PatrickAlphaC to comment on my questions (especially 1st) and will push the linting fix etc...

[PatrickAlphaC]

I like your suggestion for your point 1 @the-caliber!

2. We could just have a section called security reviews and have the difference between internal and external? It just feels like a pretty big violation of DRY (hehe) if we put the same information about manual reviews in 2 places. Internal or external, manual reviews are the same. Maybe we just use the manual reviews section of the external security reviews. What do you think?

3. Formatting PR is merged now!

[the-caliber]

Hi, Thanks!

So I will move the manual-review.md to the external security reviews/ as discussed in the first point - Correct me if i assumed something different.

I noticed that the SUMMARY.md.main in this branch (develop) needs to be fixed for security-testing section - I will correct it soon so that it wont create a problem while giving references in the manual-review.md from the external security reviews/

Currently some rebasing needs to be done. so that i can check the local mdbook previews.

will update here on this soon.

[the-caliber]

Hey @mattaereal , If you are seeing this,

To which md file the outline should be updated when making PR?

the contributing doc and PR template suggest that If you're modifying the general outline, make sure to use src/config/SUMMARY.develop and not src/SUMMARY.md

While locally hosting the mdbook, making changes to SUMMARY.md.develop does not reflect in the hosted mdbook. but when i make changes to the SUMMARY.md it reflects the changes (changes about outline/index) in the hosted mdbook.

[mattaereal]

This is a good question, and something we need to improve. Locally, using SUMMARY.md is what mdbook uses by default, but since we have different deployments, to keep track of them and not mixing them up, depending on the branch a different .md gets pulled from src/config.

So, to see your local changes summary.md is ok, but to reflect them on the correct branch, you need to modify develop's one.

[mattaereal]

Then when I move from dev to main, I replicate only the SUMMARY structure of the framework we're pushing to .org.

[the-caliber]

I see, Thanks for clarifying!

[the-caliber]

Hi @PatrickAlphaC Now it should reflect the changes in this vercel preview deployments.

I have edited manual-review.md to take care of formatting and spellings with markdownlint-cli2 and aspell respectively.
(Not sure why the file looks off(only in github files changed section) when not rendered. I think its because i am using code snippet in the list item. But when rendered/previewed it seems good.)

Moved manual-review.md to the 'external-security-reviews/smart-contracts'
Also updated the SUMMARY.md.develop to reflect the security testing correct outline.

I understand that the Automated review can't be in the manual review so i have just added references to it. I thought about adding a note similar to "automated review is not part of manual review" but generally we are assuming the viewer already know that. i think removing the Automated review can also make sense. let me know your thoughts.

[mattaereal]

Added a few suggestions! I accidentally broke the format by suggesting something that did not allow me to complete through the web ui 😓 .

Also, remember the PR template asks you to modify src/config/SUMMARY.develop, not SUMMARY.md

[the-caliber]

Hi,

Can you point out what are the suggestions you added ? Currently unable to see them in this thread. Are these in the PR I am making or somewhere else.

Also noted that there's a conflict, is it related to the thing you are saying accidentally broke the format by suggesting something that did not allow me to complete through the web ui ? Seems, no.

PS: saw the discord messages, You are talking about add mermaid syntax and replace your images of several type of graphs for mermaid compatible ones. Still, wondering where you made them(suggestions).

---

Also, remember the PR template asks you to modify src/config/SUMMARY.develop, not SUMMARY.md

Yes, I remember this. In current PR i made changes in both SUMMARY.md.develop and SUMMARY.md because the SUMMARY.md was behind (I know it should be only used for local testing/hosting mdbook as you said). So I thought it is a good practice to keep it updated because when others will pull the repo to locally test it they will be confused by seeing differences in the outline.

Please let me know if keeping the SUMMARY.md.develop updated (and adding it in PR) creates any problems.

[mattaereal]

Hi,

Can you point out what are the suggestions you added ? Currently unable to see them in this thread. Are these in the PR I am making or somewhere else.

Also noted that there's a conflict, is it related to the thing you are saying accidentally broke the format by suggesting something that did not allow me to complete through the web ui ? Seems, no.

PS: saw the discord messages, You are talking about add mermaid syntax and replace your images of several type of graphs for mermaid compatible ones. Still, wondering where you made them(suggestions).

Also, remember the PR template asks you to modify src/config/SUMMARY.develop, not SUMMARY.md

Yes, I remember this. In current PR i made changes in both SUMMARY.md.develop and SUMMARY.md because the SUMMARY.md was behind (I know it should be only used for local testing/hosting mdbook as you said). So I thought it is a good practice to keep it updated because when others will pull the repo to locally test it they will be confused by seeing differences in the outline.

Please let me know if keeping the SUMMARY.md.develop updated (and adding it in PR) creates any problems.

Huh, this is weird, it is native functionality of GitHub. Send me a screenshot of the notifications of this thread please. You should be able to see them all above my last comment, and even get notifications from them.

Here's a direct link to the files where you should also see them as well:
https://github.com/security-alliance/frameworks/pull/183/files/4b3eedfde8bbbc692160f8eed4d828612c26ec4c

[mattaereal]

Author

You should pull from develop before pushing the latest changes, since it gets modified all the time, and it will flag conflicts! Do it before asking me to merge, and after you have solved my comments

[the-caliber]

Hi @mattaereal @Robert-MacWha , Thanks for the review and suggestions.

I will go through the suggestions soon!

I will go ahead and start commiting most of the changes robert suggested, but there are some you'll need to address yourself.

thanks for that! @mattaereal , I will take a look into remaining ones. I can see you are still resolving some things so i will wait for some time!

[mattaereal]

Ok, I finished! You can go ahead and solve the other conversations :)

[the-caliber]

Thanks! I will go through them.

[the-caliber]

I have updated the things. I will comment about changes/suggestions applied soon and re-request the review!

[mattaereal]

Make sure to resolve all conversations after reaching consensus!

[the-caliber]

I have resolved the open conversations. still, there are 3 open conversations. I have commented on them, once i get your comment/reaction, i will resolve them as well.

Please feel free to reopen any resolved conversation if you think!
Additionally I have added few comments on already resolved conversations by @mattaereal for clarity.

[the-caliber]

One important question about usage of indentation for the list item's code snippets:

As you can see I am using most of the code blocks (solidity, mermaid, minimal md file) as a part of numbered list items, so most of them are indented. Now with some recent modification from Robert the The minimal markdown file for taking notes might look like this: is out of the indentation, so is the actual markdown code snippet.

Should I still keep using indentation for other snippets or they should be taken out of indentation ??
(My views are , Out of indentation looks good (since it gets more space), but it then creates visual difference where the code snippet looks little out of the list item.)

Both works for me, Maybe we just need to follow one format!

[mattaereal]

Ident everything properly for readability please :)

[Robert-MacWha]

One important question about usage of indentation for the list item's code snippets:

As you can see I am using most of the code blocks (solidity, mermaid, minimal md file) as a part of numbered list items, so most of them are indented. Now with some recent modification from Robert the The minimal markdown file for taking notes might look like this: is out of the indentation, so is the actual markdown code snippet.

Should I still keep using indentation for other snippets or they should be taken out of indentation ?? (My views are , Out of indentation looks good (since it gets more space), but it then creates visual difference where the code snippet looks little out of the list item.)

Both works for me, Maybe we just need to follow one format!

I was just making suggestions from within github's review UI lol - it doesn't do a great job at preserving style. Sorry if I messed up something by accident!