sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.12k stars 1.29k forks source link

Audit how characters are escaped in batch spec and see if there's room to improve user feedback #51314

Open courier-new opened 1 year ago

courier-new commented 1 year ago

Recently, a customer ran into a gnarly edge casee when specifying a commit message in a batch spec that started with a dash. It turned out that since the commit message was being passed to git commit -m "bare", the starting dash was being misinterpreted by the shell as misformatted args. The issue proved particularly challenging to debug because there was a lack of specific output from gitserver about the problem encountered in the error logs:

{
  "SeverityText": "ERROR",
  "Timestamp": 1681915465874079451,
  "InstrumentationScope": "gitserver.createCommitFromPatch",
  "Caller": "server/patch.go:234",
  "Function": "github.com/sourcegraph/sourcegraph/cmd/gitserver/server.(*Server).createCommitFromPatch",
  "Body": "Failed to commit patch.",
  "Resource": {
    "service.name": "gitserver",
    "service.version": "5.0.1",
    "service.instance.id": "gitserver-0"
  },
  "Attributes": {
    "repo": "github.com/ncino/slc-chugger-regression",
    "baseCommit": "477070efe3eaaf84959a2babb5caab0ebb79eceb",
    "targetRef": "refs/heads/trufflehogUpdate",
    "output": ""
  }
}

The "output": "" piece is part of what's unideal here. Typically, if a command fails to run, we still get an output, and we surface the exact failed command in the UI to provide clearer feedback to the user about what's happening. However, it seems that some misformatted commands don't result in any output from gitserver for one reason or another, despite the fact that running this command does normally result in an error:

$ git commit -m - Oops
error: pathspec 'Oops' did not match any file(s) known to git

This issue is based on this single anecdote, but to better enable users to debug misbehaving scripts or other fields in specs that are directly used in shell commands, we could benefit from conducting a more extensive audit of escape patterns in batch specs. We should seek to investigate:

github-actions[bot] commented 1 year ago

Hey, @sourcegraph/batchers (@eseliger @courier-new @adeola-ak @BolajiOlajide @Piszmog @st0nebraker @ryphil @chrispine @danielmarquespt) - we have been mentioned. Let's take a look.