fix: harden cookie flags, sanitize Genie markdown output, fix remote tunnel#216
fix: harden cookie flags, sanitize Genie markdown output, fix remote tunnel#216
Conversation
pkosiec
force-pushed
the
pkosiec/impr-security
branch
from
f8c9a2e to
e217d79
Compare
- Set httpOnly and secure flags on dev-tunnel-id cookie - Add DOMPurify sanitization to Genie chat message markdown rendering Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
pkosiec
force-pushed
the
pkosiec/impr-security
branch
from
e217d79 to
6ef9a96
Compare
pkosiec
changed the title
Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
pkosiec
force-pushed
the
pkosiec/impr-security
branch
from
a95a2fe to
da0912c
Compare
| "build:app": "tsdown --out-dir build server/index.ts && cd client && npm run build", | ||
| "build:server": "tsdown --out-dir build server/index.ts", | ||
| "install": "cd client && npm ci && cd ..", | ||
| "install": "cd client && npm install && cd ..", |
There was a problem hiding this comment.
CC @MarioCadenas we need to revert that, otherwise the pnpm deploy:playground doesn't work. The dependencies need to be downloaded on runtime side overriding the local ones (from the npm registry proxy)
There was a problem hiding this comment.
so? npm ci sould work anyway, what's the problem with that?
There was a problem hiding this comment.
Nope, because the lock file doesn't match - when pnpm deploy:playground runs, the deployed package.json has rewritten dependencies (file: tarballs replaced with registry versions), which means the lockfile no longer matches
There was a problem hiding this comment.
are we running this one on ci at any moment? or just locally?
| return /^[^./]/.test(id) || id.includes("/node_modules/"); | ||
| }, | ||
| tsconfig: "./tsconfig.json", | ||
| copy: [ |
There was a problem hiding this comment.
This was needed to resolve an issue that crashed the app when running the tunnel:
node:fs:442
return binding.readFileUtf8(path, stringToFlags(options.flag));
^
Error: ENOENT: no such file or directory, open '/app/python/source_code/node_modules/@databricks/appkit/dist/plugins/server/remote-tunnel/index.html'
at Object.readFileSync (node:fs:442:20)
at file:///app/python/source_code/node_modules/@databricks/appkit/dist/plugins/server/remote-tunnel/remote-tunnel-manager.js:121:18
at middleware (file:///app/python/source_code/node_modules/@databricks/appkit/dist/plugins/server/remote-tunnel/remote-tunnel-controller.js:60:51) {
errno: -2,
code: 'ENOENT',
syscall: 'open',
path: '/app/python/source_code/node_modules/@databricks/appkit/dist/plugins/server/remote-tunnel/index.html'
}
Node.js v22.16.0
prepare-pr-template.ts rewrites dependencies to file: tarballs, which will never match the lockfile copied from template/ — npm ci will always fail here, so npm install is required. Signed-off-by: Pawel Kosiec <pawel.kosiec@databricks.com>
| - name: Install template dependencies | ||
| working-directory: pr-template | ||
| run: npm ci | ||
| run: npm install |
There was a problem hiding this comment.
prepare-pr-template.ts rewrites dependencies to file: tarballs, which will never match the lockfile copied from template - so we need to revert this
There was a problem hiding this comment.
true, but I guess now we have fixed deps it should be fine
There was a problem hiding this comment.
No - see this failure: https://github.com/databricks/appkit/actions/runs/23590462691/job/68694129912
The prepare-pr-template.ts script rewrites deps from registry specifiers to file paths. The lockfile still references the original registry URLs, so npm ci will fail because the specifiers don't match - regardless of whether versions are pinned.
There was a problem hiding this comment.
I meant keeping npm install sorry, I understand we can't keep npm ci in there
2 participants
Summary
dev-remotetunnel crash: copy HTML template files (index.html,wait.html,denied.html) todist/during build via tsdowncopyconfig instead of a shellcpcommand — they were missing because tsdown only transpiles TypeScripthttpOnly: trueandsecure: trueon thedev-tunnel-idcookie in the remote tunnel manager. The cookie is only read server-side (HTTP requests + WebSocket upgrades), never by client-side JS.DOMPurifysanitization to Genie chat message markdown rendering (dangerouslySetInnerHTML) as defense-in-depth against potential XSS via API content.npm ciback tonpm install—prepare-pr-template.tsrewrites dependencies tofile:tarballs, which will never match the lockfile copied fromtemplate/.Test plan
dist/plugins/server/remote-tunnel/