Introduce --allow-unsafe option and set CSP header
#1162
Conversation
c-bata
changed the title
--allow-unsafe--allow-unsafe option and set CSP header
c-bata
force-pushed
the
allow-unsafe-option
branch
from
1da2b80 to
5b8848b
Compare
porink0424
left a comment
porink0424
left a comment
There was a problem hiding this comment.
The overall direction looks good to me 👍 I just left two comments.
| </Box> | ||
| </Box> | ||
| {renderIframe()} | ||
| {llmEnabled && renderIframe()} |
There was a problem hiding this comment.
I think it’s a good call not to invoke renderIframe() when the LLM isn’t enabled. It might also be a good idea to take this opportunity to fix the part that calls renderIframe() of useTrialFilterQuery.
| else: | ||
| # CSP header | ||
| if llm_provider is not None: | ||
| script_src_str = "script-src 'self' 'unsafe-inline' 'unsafe-eval'" |
There was a problem hiding this comment.
Do you think there's a way to avoid specifying discouraged unsafe-inline or unsafe-eval? But I also realize fixing this could be a lot of work, so not spending that effort right now would also be a reasonable.
There was a problem hiding this comment.
Thank you for your review!
I tried removing 'unsafe-inline', but it caused the CSS to break completely, likely due to the way MUI handles styling. The 'unsafe-eval' directive was intentionally included because the LLM integration relies on evaluating JavaScript functions.
c-bata
force-pushed
the
allow-unsafe-option
branch
from
266da87 to
5909c17
Compare
|
@porink0424 Thank you for your review. I applied your suggestion in ae575d5. Please take a look again. |
2 participants
Contributor License Agreement
This repository (
optuna-dashboard) and Goptuna share common code.This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.
Reference Issues/PRs
None
What does this implement/fix? Explain your changes.
--allow-unsafeCLI/TOML option