I would recommend sticking with the Bitcoin structure for a couple of reason. Primarily the part I mean is that are effectively parameters to another script (like the lock script is a parameter to the P2SH logic) should be wrapped as a binary blob in a PUSHDATA operation as a single stack item. They can then be passed to whatever needs them for parsing and execution.
One big reason for this is that we will likely have several use cases for pushing scripts (that may be either executed or inspected) within other scripts. An example is here. Note that wrapping the script in a PUSHDATA operation replaces the need for the seperator opcode since the data is delimited, and has the side effect of pushing the item onto the stack at the same time. If we are going to use the notion of defining scripts in scripts for multiple use cases it would be good if we have a consistent mechanism across the board. The best way I can see to do that is to keep is as ‘scriptish’ as possible.
I don’t understand the need for the tx_in_signable_string? The locking script would usually include a CHECKSIG operation, if it doesn’t I agree it’s insecure, but that is also true of a non-P2SH script. If we allow those why would we treat P2SH any differently? The attack is the same in both cases, wait until the solution to the hash puzzle is presented in a transaction, suppress the transaction and make a new transaction that spends the coin with the revealed solution.
If implementing it the Bitcoin way one variation I would consider is a more explicit marker. For backward compatibility purposes the “marker” that tell the script engine to use P2SH logic is the script pattern “HASH160 <script_hash> EQUALVERIFY”. We could make this more explicit with an OP code instead e.g. “OP_EXEC_SCRIPT_IF_HASH <script_hash>” (name of op code is open to debate, I was just being explicit about what it’s supposed to do.