anchors are dropped by language server (in vscode extension)

[fopinappb]

I'm opening this issue following up on this slack thread

When using the vscode extension, I saw that shortlink was used in the problem details to link to rule details.

As I tried to use it with a page that list all my custom rules and used #rule-id for direct links, it didn't work. Anything after # (or ?) is dropped.

I've tested in intellij out of curiosity and those are kept there which means the issue would be something around these lines:

semgrep/src/osemgrep/language_server/scan_helpers/Diagnostics.ml

Lines 25 to 35 in fd2717c

	let message =
	match shortlink with
	(* IntelliJ doesn't display code descriptions:/ so we must insert them here *)
	| Some s when is_intellij ->
	message
	^ Printf.sprintf "\nSemgrep(<a href=\"%s\">%s</a>)" s check_id_str
	| _ -> message
	in
	let codeDescription =
	Option.bind shortlink (fun s ->
	Some (CodeDescription.create ~href:(Uri.t_of_yojson (`String s))))

[kopecs]

Looks like this is due to the upstream implementation of LSP we rely on: https://github.com/ocaml/ocaml-lsp/blob/556da72593c88b027a7124ae86da6cd638c7d679/lsp/src/uri0.ml#L1-L4

Although they may now have the required support: ocaml/ocaml-lsp#1387 (seems to be as-of-yet unreleased)

[fopinappb]

Interesting that query is already there and in my testing it was also dropped

Their release cadence does not seem very frequent thought 😞

Why is CodeDescription using that method to build the link yet for intellij it is just concatenated in the string?
Couldn't the same be done in codedescription (simple sprintf)?
Is it because of XSS? (meaning intellij would be vulnerable?)

[kopecs]

Based off of the comment I assume it's because IntelliJ just doesn't display the code description; I believe if we did the same for VSCode it would appear in a different location than it currently does. Given why this is happening it may be a bit longer than I had anticipated originally :/.

I would expect the editor to sanitize any HTML it chooses to render, so I don't think XSS is a concern.

cc: @ajbt200128

[ajbt200128]

ocaml/ocaml-lsp#1108 yea like @kopecs said this is a deficiency in the ocaml-lsp library, and why queries are also being dropped.

As for why intellij concatenates it, indeed it is because they don't show the code description (although there are some alternative sdks for LSP support for intellij products that may support this).

[fopina]

Not familiar at all with OCAML, is it possible to vendor/bundle in a dependency in a clean way?

or can that specific bit be moved into semgrep and fixed? (with a proper comment to be removed once dependency was updated)

I wouldn’t mind looking into if it’s more work but acceptable PR

[kopecs]

Yes, it is possible, but it is a fair amount of additional work. The main thing would be changing the dependency here on lsp to the alternative version (See dune's docs). We do something similar for pcre2 bindings.

I'll be out next week but I'm generally happy to review contributions, although I would generally prefer avoiding the need to maintain something redundant.

[fopina]

Maybe it could have been a fun exercise for a PR but it seems easier to wait for this week then hehe

ocaml/ocaml-lsp#1406 (comment)

[fopina]

The new release was made last friday and I thought I'd try bumping it myself yet versioning does not seem very semver-compatible. While it's minor bump (1.15.1 -> 1.20), there are API breaking changes.

Actually, introduced already by bumping to 1.16.1

Looking at ocaml/ocaml-lsp@1.15.1-5.0...1.16.1 quite a few changes to review 😓

dune build _build/install/default/bin/semgrep-core
File "src/osemgrep/language_server/server/Lsp_.ml", line 133, characters 4-21:
133 |     SN.Progress.Begin (WorkDoneProgressBegin.create ~message ~title ())
          ^^^^^^^^^^^^^^^^^
Error: Unbound module SN.Progress
File "src/osemgrep/cli_lsp/Lsp_subcommand.ml", lines 18-47, characters 4-10:
18 | ....Lsp.Io.Make
19 |       (struct
20 |         include Lwt
21 |
22 |         module O = struct
...
44 |               read_exactly (line :: acc) (n - String.length line)
45 |           in
46 |           read_exactly [] n
47 |       end)
Error: The functor application is ill-typed.
       These arguments:
         $S1 $S2
       do not match these parameters:
         functor (Io : ...) (Chan : $T2) -> ...
       1. Module $S1 matches the expected module type
       2. Modules do not match:
            $S2 :
            sig
              type input = Lwt_io.input_channel
              type output = Lwt_io.output_channel
              val read_line : input -> string option Lwt.t
              val write : output -> string -> unit Lwt.t
              val read_exactly : input -> int -> string option Lwt.t
            end
          is not included in
            $T2 =
            sig
              type input
              type output
              val read_line : input -> string option Io.t
              val read_exactly : input -> int -> string option Io.t
              val write : output -> string list -> unit Io.t
            end
          Values do not match:
            val write : output -> string -> unit Io.t
          is not included in
            val write : output -> string list -> unit Io.t
          The type output -> string -> unit Io.t
          is not compatible with the type output -> string list -> unit Io.t
          Type string is not compatible with type string list
          File "lsp/src/io.mli", line 26, characters 2-48:
            Expected declaration
          File "src/osemgrep/cli_lsp/Lsp_subcommand.ml", line 34, characters 12-17:
            Actual declaration
make[1]: *** [minimal-build] Error 1
make: *** [core] Error 2

As I have no clue about ocaml, I'll need to find more time to look into this but sharing already in case there's a very simple fix (and I'll close my PR)

[fopina]

hi @kopecs , ended up managed to get a working build with 1.20.0 and the help of Copilot 😄

#10702 ready for review, if you find the time

I didn't squash to make it easier to review and as it can be squashed on merge, just let me know if I should squash myself though