Skip to content

Conversation

@mm0
Copy link

@mm0 mm0 commented Oct 29, 2025

Resolves #129

Summary

This PR updates the IBC startup scripts to support TWS/Gateway version 10.37 and implements secure credential handling across all shell scripts by using environment variables.

Changes Made

Version Updates:

  • Updated TWS_MAJOR_VRSN from 1019 to 1037 across all startup scripts (Windows batch and Unix shell scripts)
  • Updated version number comments to reflect the new major version

Credential Handling Improvements:

  • Refactored credential variables (TWSUSERID, TWSPASSWORD, FIXUSERID, FIXPASSWORD) to use bash parameter expansion with empty defaults (${VAR:-})
  • Added proper quoting around TWOFA_TIMEOUT_ACTION variable to prevent word splitting issues
  • This allows scripts to inherit credentials from environment variables while maintaining backward compatibility

Path Resolution:

  • Changed telnet path in commandsend.sh from hardcoded /usr/local/bin/telnet to /usr/bin/env telnet for better portability across different system configurations

Files Modified

  • resources/StartGateway.bat
  • resources/StartTWS.bat
  • resources/commandsend.sh
  • resources/gatewaystart.sh
  • resources/twsstart.sh
  • resources/twsstartmacos.sh

Testing

Scripts maintain backward compatibility with existing configurations while providing better support for environment-based credential management.

No testing for windows or mac was done

ci added 3 commits October 28, 2025 23:21
…mmand-line arguments

Addresses security vulnerability where credentials were visible in process
listings (ps -ef) when passed as command-line arguments.

Changes:
- Modified DefaultLoginManager to read credentials from environment variables
  (TWSUSERID, TWSPASSWORD, FIXUSERID, FIXPASSWORD) with priority over args
- Updated ibcstart.sh to export credentials as environment variables before
  launching Java process
- Updated StartIBC.bat to set credentials as environment variables before
  launching Java process
- Simplified Java invocation in scripts - credentials no longer passed as
  command-line arguments

Benefits:
- Credentials no longer visible in process listings
- Enables secure credential storage using file permissions
- Maintains backward compatibility with existing configurations
- Consistent behavior across Linux, macOS, and Windows platforms

### Running the Scripts

**Linux/macOS:**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't right. macOS have their own scripts, see twsstartmacos.sh and gatewaystartmacos.sh

TWSUSERID=
TWSPASSWORD=
TWSUSERID=${TWSUSERID:-}
TWSPASSWORD=${TWSPASSWORD:-}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't these changes also be applied to gatewaystartmacos.sh?

@rlktradewright
Copy link
Member

Apologies for not responding to this PR before now. There are quite a lot of points to make, and the net result is that I will not be accepting this in is current form.

First of alll, this is arguably three different sets of unrelated changes, so it should be three PRs, not one. Mixing up unrelated changes just makes everything more difficult (except in the rare case where all the changes are accepted as they stand). Making multiple PRs is no more difficult that making one, so there's really no reason for doing it. We can quibble about the README changes being directly related to the credential handling changes, but documentation changes are very likely to cause dicsussion and, I feel, are better separated out.

Next I'm not keen on the environment variable approach to storing credentials. I don't have any intrinsic objection to it, but it adds complexity and in my opinion doesn't add any security. If we want to avoid credentials being shown in process listings we can just remove the option of setting them in the start scripts, which I've been mulling over for a long time. The only reason for still being able to set them there is that it's been there since 2007, and I suppose there's a chance that someone still does this. I don't recall the reason for this in the first place, I expect someone asked for the ability to be able to pass credentials via the main entrypoint and we didn't notice the implications at the time (ah yes, it was requested on 6 Feb 2007. But I really think it's time that was removed from the scripts (though I would still leave the option to set the credentials via args when calling IBC from another program rather than a script.

Now, I'm perfectly open to offering the ability to store credentials in environment variables as an alternative to the config file, but the way to do this is to use a custom LoginManager class which extends ibcalpha.ibc.LoginManager: the environment variable handling would then be implemented there. However more work is needed to provide a mechanism to actually invoke the right class, which might involve a custom classloader or manipulating the classpath. This needs more thought. But I would enviasage a new config.ini setting something like this:

CustomLoginManager=<path to JAR file>/<class name>

If this setting exists, IBC would load the specified class rather than the default login manager.

That'll do for the moment. I'll try to give some detailed feedback on the file changes soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Securing cleartext user-id/pwd

3 participants