Hi, I made this DIY preamp and not sure if I have the grounding correct.
I am grounding the RED input / output together and the BLACK input / output. Is this correct? Do I star ground all together?
I tried not connecting the grounds (I think) and got a loud hummm. It went away when I connected it as shown in the pic.

As you simply have a resistive attenuator in each line there is no need to connect the grounds of the two channels together. As you found, you do need to connect the input and output grounds for each channel because this provides an intact ground reference for the signal.

When you have to ground things, a star ground is a good idea. When you don't have to ground things, because they are grounded elsewhere and safety is not an issue, it is best to leave them floating. This avoids hum loops.

A star ground is where all (or almost all) the ground connections in an amplifier are grounded at the same point. This may, or may not, be a chassis connection too.

If the pot works OK as it is then a plastic case should be OK. If you get hum or interference pickup, as your wires appear to be unscreened, then you would need a metal case. You should connect the grounds on one channel to the case. Then the case will act as a screen.

When a unit is powered, then the case should connect to the safety ground. The signal ground may be kept separate, or partially separate.