getFirstString can only return a string or undefined so this could be something like:

if (!uri.path) {
  throw new Error("no path was provided")
}

Will do

If this function can take an object we should update the types on it I think.

It can't.

I was just testing this logic

  if (typeof uri.path !== "string") {
    throw new Error(
      `Could not compute fsPath from given uri. Expected path to be of type string, but was of type ${typeof uri.path}.`,
    )
  }

Whoops I mean remove @ts-expect-error then update pathToFsPath

export function pathToFsPath(path: string | string[] | object, keepDriveLetterCasing = false): string

Or in other words if pathToFsPath can accept an object the types should reflect that.

Although I don't think we should make it accept a object so it probably makes more sense to remove this test. We don't currently have anywhere in the code where this arg might be an object (I think! I could be wrong.)

Sorry! I think my first comment was confusing. I added this piece of logic

  if (typeof uri.path !== "string") {
    throw new Error(
      `Could not compute fsPath from given uri. Expected path to be of type string, but was of type ${typeof uri.path}.`,
    )
  }

to make sure we handled the scenario where something other than a string was being passed in. I think there is somewhere in the codebase where we're passing in something that we typecast as a string but I'm not sure 🤔

In order to test that, I need to make path something other than a string. I chose object. And that's why I'm using
// @ts-expect-error We need to check other types.

Right! What I mean is that there are two scenarios:

1. You should be able to pass non-string arguments to pathToFsPath.
2. You should not be able to pass non-string arguments to pathToFsPath.

If the first is true then the types in the function signature should reflect this (with any or whatever the list of possible types are). So this would entail changing the signature to something like path: any instead of path: string. Otherwise, if we can't trust our types then we'd need to add type guards in every function! (And that means testing every function with invalid arguments too---that'd be rough lol.)

If the second is true then the code that is calling pathToFsPath with invalid arguments is incorrect/broken and we will need to fix it. Usually TypeScript catches this for us but there are some cases where the data at some boundary comes in as any so we have to look out for those. I don't think that's the case here though.

In our case the data comes in as string | string[] | qs.ParsedQs | qs.ParsedQs[] | undefined. We then check that the argument is a string before calling pathToFsPath so the second option should suit our purposes:

https://github.com/cdr/code-server/blob/717eaa64704f69c763d6fa6502d7b1c1ba9d628f/src/node/routes/static.ts#L19-L22

https://github.com/cdr/code-server/blob/ddee4f748c693c26b19af964994907c7eeef6828/src/node/routes/vscode.ts#L66-L68

https://github.com/cdr/code-server/blob/ddee4f748c693c26b19af964994907c7eeef6828/src/node/routes/vscode.ts#L76-L78

So why do we get an error? I'm not 100% sure but I think what's happening is the tool is thinking that since req.query is an object it can be manipulated in between the if and the function call which can turn it into an invalid argument (like if someone did req.query.path = {}).

TypeScript is able to detect this and show an error but whatever tool reported this apparently can't.

If that's the case then we'll need to extract it to a constant first, e.g.:

const path = getFirstString(req.query.path)
if (path) { 
  res.set("Content-Type", getMediaMime(path)) 
  res.send(await fs.readFile(pathToFsPath(path))) 

Then our pathToFsPath can purely accept string without having to do any argument manipulation itself.

Can remove.