Trust the default cell

[krassowski]

Empty cells should be trusted by default. This accompanies a larger work in JupyterLab (jupyterlab/jupyterlab#14345) aimed at preserving trust behaviour of classing notebook (jupyterlab/jupyterlab#14025).

Addresses one point in jupyterlab/jupyterlab#14347.

An alternative (as discussed in the linked PR) is to always add trusted: true metadata to code cells which do not have any outputs (which could be implemented in sharedModel.insertCell) with the pros/cons:

• (+) less repetitive
• (+) harder to forget about trust downstream
• (-) less secure in case if the output gets populated immediately after cell creation (e.g. by unaware extension developer) with result not controlled by the user

Given that security implications I am proposing the more conservative approach of doing it manually each time for now, but I think we could as well automate this in the future (with a big disclaimer in extension upgrade guide).

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[krassowski]

In general it would be good to get your thoughts on jupyterlab/jupyterlab#14347 but I have a suspicion that this is JupyterLab-side issue and "Trust Notebook" implementation will need to be adjusted to work well with RTC.

[davidbrochart]

Thanks @krassowski.
Should we do the same here?

[krassowski]

Should we do the same here?

No, not in general as create_ycell can be called with an existing cells, e.g. here:

jupyter_ydoc/jupyter_ydoc/ynotebook.py

Lines 263 to 264 in 74f4693

	for cell in cells:
	self._ycells.append(t, self.create_ycell(cell))

and that may contain an outputs which may need sanitising.

[davidbrochart]

Yes, not in general but maybe after checking that the cell has no output?

[krassowski]

I would lean towards "still no", because if it comes from an untrusted notebook (with a code cell with outputs that is not trusted) if we mark other code cells with no outputs we would be changing notebook's metadata in flight inducing an unnecessary noise and signature change later on.

But it's not a hard no.

[davidbrochart]

Sounds good, thanks for the explanation!

[fcollonval]

Thanks @krassowski

[fcollonval]

CI failure is not related

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

[fcollonval]

@meeseeksdev please backport to 0.2.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 0.2.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 3b3a4af50ea46abd886076b69b93ee0dab6b05c5

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #161: Trust the default cell'

4. Push to a named branch:

git push YOURFORK 0.2.x:auto-backport-of-pr-161-on-0.2.x

5. Create a PR against branch 0.2.x, I would have named this PR:

"Backport PR #161 on branch 0.2.x (Trust the default cell)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

[krassowski]

Thank you for the backport and release!